Skip to content

Commit

Permalink
Don't dynamically allocate arrays on the stack
Browse files Browse the repository at this point in the history
  • Loading branch information
@Asmod4n
Asmod4n committed Jun 3, 2021
1 parent 85c2ce9 commit 25edeb0
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 64 deletions.
2 changes: 1 addition & 1 deletion build_config.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
toolchain :gcc
enable_debug
conf.enable_sanitizer "address,undefined,leak"
conf.cc.flags << '-fno-omit-frame-pointer' << '-ggdb'
conf.cc.flags << '-fno-omit-frame-pointer' << '-g -ggdb'
conf.enable_debug
conf.enable_bintest
conf.enable_test
Expand Down
105 changes: 42 additions & 63 deletions src/mrb_hiredis.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,21 @@
static void
mrb_hiredis_check_error(const redisContext *context, mrb_state *mrb)
{
if (context->err != 0) {
switch (context->err) {
case REDIS_ERR_IO:
mrb_sys_fail(mrb, context->errstr);
break;
case REDIS_ERR_EOF:
mrb_raise(mrb, E_EOF_ERROR, context->errstr);
break;
case REDIS_ERR_PROTOCOL:
mrb_raise(mrb, E_HIREDIS_ERR_PROTOCOL, context->errstr);
break;
case REDIS_ERR_OOM:
mrb_raise(mrb, E_HIREDIS_ERR_OOM, context->errstr);
break;
default:
mrb_raise(mrb, E_HIREDIS_ERROR, context->errstr);
}
switch (context->err) {
case REDIS_ERR_IO:
mrb_sys_fail(mrb, context->errstr);
break;
case REDIS_ERR_EOF:
mrb_raise(mrb, E_EOF_ERROR, context->errstr);
break;
case REDIS_ERR_PROTOCOL:
mrb_raise(mrb, E_HIREDIS_ERR_PROTOCOL, context->errstr);
break;
case REDIS_ERR_OOM:
mrb_raise(mrb, E_HIREDIS_ERR_OOM, context->errstr);
break;
default:
mrb_raise(mrb, E_HIREDIS_ERROR, context->errstr);
}
}

Expand Down Expand Up @@ -179,23 +177,15 @@ mrb_redisCommandArgv(mrb_state *mrb, mrb_value self)
mrb_int argc = 0;

mrb_get_args(mrb, "n*", &command, &mrb_argv, &argc);
argc++;

const char *argv[argc];
size_t argvlen[argc];
mrb_int command_len;
argv[0] = mrb_sym2name_len(mrb, command, &command_len);
argvlen[0] = command_len;

mrb_int argc_current;
for (argc_current = 1; argc_current < argc; argc_current++) {
mrb_value curr = mrb_str_to_str(mrb, mrb_argv[argc_current - 1]);
argv[argc_current] = RSTRING_PTR(curr);
argvlen[argc_current] = RSTRING_LEN(curr);
}

const char **argv = NULL;
size_t *argvlen = NULL;
mrb_hiredis_generate_argv_argc_array(mrb, command, mrb_argv, &argc, &argv, &argvlen);

errno = 0;
redisReply *reply = redisCommandArgv(context, argc, argv, argvlen);
mrb_free(mrb, argv);
mrb_free(mrb, argvlen);
if (likely(reply != NULL)) {
mrb_value reply_cptr_value = mrb_cptr_value(mrb, reply);
return mrb_ensure(mrb, mrb_redisCommandArgv_cb, reply_cptr_value, mrb_redisCommandArgv_ensure, reply_cptr_value);
Expand Down Expand Up @@ -224,20 +214,6 @@ mrb_redisAppendCommandArgv(mrb_state *mrb, mrb_value self)
mrb_int argc = 0;

mrb_get_args(mrb, "n*", &command, &mrb_argv, &argc);
argc++;

const char *argv[argc];
size_t argvlen[argc];
mrb_int command_len;
argv[0] = mrb_sym2name_len(mrb, command, &command_len);
argvlen[0] = command_len;

mrb_int argc_current;
for (argc_current = 1; argc_current < argc; argc_current++) {
mrb_value curr = mrb_str_to_str(mrb, mrb_argv[argc_current - 1]);
argv[argc_current] = RSTRING_PTR(curr);
argvlen[argc_current] = RSTRING_LEN(curr);
}

mrb_sym queue_counter_sym = mrb_intern_lit(mrb, "queue_counter");
mrb_value queue_counter_val = mrb_iv_get(mrb, self, queue_counter_sym);
Expand All @@ -249,9 +225,14 @@ mrb_redisAppendCommandArgv(mrb_state *mrb, mrb_value self)
}
}

const char **argv = NULL;
size_t *argvlen = NULL;
mrb_hiredis_generate_argv_argc_array(mrb, command, mrb_argv, &argc, &argv, &argvlen);

errno = 0;
int rc = redisAppendCommandArgv(context, argc, argv, argvlen);
mrb_free(mrb, argv);
mrb_free(mrb, argvlen);
if (likely(rc == REDIS_OK)) {
mrb_iv_set(mrb, self, queue_counter_sym, mrb_fixnum_value(queue_counter));
return self;
Expand Down Expand Up @@ -308,7 +289,7 @@ mrb_redisGetReply(mrb_state *mrb, mrb_value self)
errno = 0;
int rc = redisGetReply(context, (void **) &reply);
if (likely(rc == REDIS_OK)) {
if (reply != NULL) {
if (likely(reply != NULL)) {
mrb_redisGetReply_cb_data cb_data = { self, reply };
mrb_value cb_data_val = mrb_cptr_value(mrb, &cb_data);
return mrb_ensure(mrb, mrb_redisGetReply_cb, cb_data_val, mrb_redisGetReply_ensure, cb_data_val);
Expand Down Expand Up @@ -532,7 +513,7 @@ mrb_redisConnectCallback(const struct redisAsyncContext *async_context, int stat
}
}

MRB_INLINE void
MRB_INLINE mrb_value
mrb_hiredis_setup_async_context(mrb_state *mrb, mrb_value self, mrb_value callbacks, mrb_value evloop, redisAsyncContext *async_context)
{
mrb_iv_set(mrb, self, mrb_intern_lit(mrb, "@callbacks"), callbacks);
Expand Down Expand Up @@ -560,6 +541,8 @@ mrb_hiredis_setup_async_context(mrb_state *mrb, mrb_value self, mrb_value callba
async_context->ev.cleanup = mrb_hiredis_cleanup;
redisAsyncSetDisconnectCallback(async_context, mrb_redisDisconnectCallback);
redisAsyncSetConnectCallback(async_context, mrb_redisConnectCallback);

return self;
}

static mrb_value
Expand Down Expand Up @@ -590,8 +573,7 @@ mrb_redisAsyncConnect(mrb_state *mrb, mrb_value self)
if (likely(async_context != NULL)) {
mrb_data_init(self, async_context, &mrb_redisAsyncContext_type);
if (likely(async_context->c.err == 0)) {
mrb_hiredis_setup_async_context(mrb, self, callbacks, evloop, async_context);
return self;
return mrb_hiredis_setup_async_context(mrb, self, callbacks, evloop, async_context);
} else {
mrb_hiredis_check_error(&async_context->c, mrb);
return mrb_false_value();
Expand Down Expand Up @@ -661,20 +643,6 @@ mrb_redisAsyncCommandArgv(mrb_state *mrb, mrb_value self)
mrb_value block = mrb_nil_value();

mrb_get_args(mrb, "n*&", &command, &mrb_argv, &argc, &block);
argc++;

const char *argv[argc];
size_t argvlen[argc];
mrb_int command_len;
argv[0] = mrb_sym2name_len(mrb, command, &command_len);
argvlen[0] = command_len;

mrb_int argc_current;
for (argc_current = 1; argc_current < argc; argc_current++) {
mrb_value curr = mrb_str_to_str(mrb, mrb_argv[argc_current - 1]);
argv[argc_current] = RSTRING_PTR(curr);
argvlen[argc_current] = RSTRING_LEN(curr);
}

errno = 0;
int rc = REDIS_ERR;
Expand All @@ -685,7 +653,13 @@ mrb_redisAsyncCommandArgv(mrb_state *mrb, mrb_value self)
memcpy(block_p, &block, sizeof(mrb_value));
mrb_value block_cb_data = mrb_obj_value(block_cb_data_p);
mrb_iv_set(mrb, block_cb_data, mrb_intern_lit(mrb, "block"), block);
const char **argv = NULL;
size_t *argvlen = NULL;
mrb_hiredis_generate_argv_argc_array(mrb, command, mrb_argv, &argc, &argv, &argvlen);
rc = redisAsyncCommandArgv(async_context, mrb_redisCallbackFn, block_cb_data_p, argc, argv, argvlen);
mrb_free(mrb, argv);
mrb_int command_len = argvlen[0];
mrb_free(mrb, argvlen);
if (likely(rc == REDIS_OK)) {
if ((command_len == 9 && strncasecmp(argv[0], "subscribe", command_len) == 0)||
(command_len == 10 && strncasecmp(argv[0], "psubscribe", command_len) == 0)) {
Expand All @@ -711,7 +685,12 @@ mrb_redisAsyncCommandArgv(mrb_state *mrb, mrb_value self)
}
}
} else {
const char **argv = NULL;
size_t *argvlen = NULL;
mrb_hiredis_generate_argv_argc_array(mrb, command, mrb_argv, &argc, &argv, &argvlen);
rc = redisAsyncCommandArgv(async_context, NULL, NULL, argc, argv, argvlen);
mrb_free(mrb, argv);
mrb_free(mrb, argvlen);
}
if (likely(rc == REDIS_OK)) {
return self;
Expand Down
50 changes: 50 additions & 0 deletions src/mrb_hiredis.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,56 @@ static const struct mrb_data_type mrb_redisCallbackFn_cb_data_type = {
"$i_mrb_redisCallbackFn_cb_data_type", mrb_free
};

typedef struct {
const mrb_sym command;
const mrb_value *mrb_argv;
mrb_int *argc_p;
const char ***argv_p;
size_t **argvlen_p;
} mrb_hiredis_generate_argv_argc_array_cb_data;

MRB_INLINE mrb_value
mrb_hiredis_generate_argv_argc_array_cb(mrb_state *mrb, const mrb_value command_argv_argc)
{
mrb_hiredis_generate_argv_argc_array_cb_data *cb_data = (mrb_hiredis_generate_argv_argc_array_cb_data *) mrb_cptr(command_argv_argc);
*cb_data->argv_p = NULL;
*cb_data->argvlen_p = NULL;
*cb_data->argc_p += 1;
if (likely((*cb_data->argc_p <= SIZE_MAX / sizeof(const char **)) && (*cb_data->argc_p <= SIZE_MAX / sizeof(size_t *)))) {
const char **argv = *cb_data->argv_p = mrb_malloc(mrb, *cb_data->argc_p * sizeof(const char **));
size_t *argvlen = *cb_data->argvlen_p = mrb_malloc(mrb, *cb_data->argc_p * sizeof(size_t *));
mrb_int command_len;
argv[0] = mrb_sym2name_len(mrb, cb_data->command, &command_len);
argvlen[0] = command_len;

mrb_int argc_current;
for (argc_current = 1; argc_current < *cb_data->argc_p; argc_current++) {
mrb_value curr = mrb_str_to_str(mrb, cb_data->mrb_argv[argc_current - 1]);
argv[argc_current] = RSTRING_PTR(curr);
argvlen[argc_current] = RSTRING_LEN(curr);
}
} else {
mrb_raise(mrb, E_RUNTIME_ERROR, "Cannot allocate argv array");
}

return mrb_nil_value();
}

static void
mrb_hiredis_generate_argv_argc_array(mrb_state *mrb, const mrb_sym command, const mrb_value *mrb_argv, mrb_int *argc_p, const char ***argv_p, size_t **argvlen_p)
{
mrb_hiredis_generate_argv_argc_array_cb_data cb_data = {command, mrb_argv, argc_p, argv_p, argvlen_p};
mrb_value cb_data_obj = mrb_cptr_value(mrb, &cb_data);

mrb_bool error;
mrb_value result = mrb_protect(mrb, mrb_hiredis_generate_argv_argc_array_cb, cb_data_obj, &error);
if (unlikely(error)) {
mrb_free(mrb, *argv_p);
mrb_free(mrb, *argvlen_p);
mrb_exc_raise(mrb, result);
}
}

typedef struct {
mrb_state *mrb;
mrb_value self;
Expand Down

0 comments on commit 25edeb0

Please sign in to comment.