From d24193630ee5f3b5fcf5fd00bcaf95106b7a9c6d Mon Sep 17 00:00:00 2001 From: frsyuki Date: Thu, 22 Apr 2010 14:46:54 +0900 Subject: [PATCH] reverts variable-length stack: avoids memory leak --- cpp/unpack.c | 16 ++++------------ msgpack/unpack_define.h | 2 +- msgpack/unpack_template.h | 12 ++++++++++++ perl/unpack.c | 3 --- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cpp/unpack.c b/cpp/unpack.c index 98c86536..34016fd1 100644 --- a/cpp/unpack.c +++ b/cpp/unpack.c @@ -44,7 +44,6 @@ struct template_context; typedef struct template_context template_context; static void template_init(template_context* ctx); -static void template_destroy(template_context* ctx); static msgpack_object template_data(template_context* ctx); @@ -216,7 +215,6 @@ bool msgpack_unpacker_init(msgpack_unpacker* mpac, size_t initial_buffer_size) void msgpack_unpacker_destroy(msgpack_unpacker* mpac) { msgpack_zone_free(mpac->z); - template_destroy(CTX_CAST(mpac->ctx)); free(mpac->ctx); decl_count(mpac->buffer); } @@ -370,7 +368,6 @@ msgpack_unpack_return msgpack_unpack(const char* data, size_t len, size_t* off, msgpack_zone* z, msgpack_object* result) { - msgpack_unpack_return ret = MSGPACK_UNPACK_SUCCESS; template_context ctx; template_init(&ctx); @@ -382,26 +379,21 @@ msgpack_unpack(const char* data, size_t len, size_t* off, int e = template_execute(&ctx, data, len, &noff); if(e < 0) { - ret = MSGPACK_UNPACK_PARSE_ERROR; - goto out; + return MSGPACK_UNPACK_PARSE_ERROR; } if(off != NULL) { *off = noff; } if(e == 0) { - ret = MSGPACK_UNPACK_CONTINUE; - goto out; + return MSGPACK_UNPACK_CONTINUE; } *result = template_data(&ctx); if(noff < len) { - ret = MSGPACK_UNPACK_EXTRA_BYTES; - goto out; + return MSGPACK_UNPACK_EXTRA_BYTES; } -out: - template_destroy(&ctx); - return ret; + return MSGPACK_UNPACK_SUCCESS; } diff --git a/msgpack/unpack_define.h b/msgpack/unpack_define.h index 71412eea..959d3519 100644 --- a/msgpack/unpack_define.h +++ b/msgpack/unpack_define.h @@ -30,7 +30,7 @@ extern "C" { #ifndef MSGPACK_EMBED_STACK_SIZE -#define MSGPACK_EMBED_STACK_SIZE 16 +#define MSGPACK_EMBED_STACK_SIZE 32 #endif diff --git a/msgpack/unpack_template.h b/msgpack/unpack_template.h index 4b8cd14f..0fbfbb78 100644 --- a/msgpack/unpack_template.h +++ b/msgpack/unpack_template.h @@ -58,9 +58,12 @@ msgpack_unpack_struct_decl(_context) { unsigned int cs; unsigned int trail; unsigned int top; + /* msgpack_unpack_struct(_stack)* stack; unsigned int stack_size; msgpack_unpack_struct(_stack) embed_stack[MSGPACK_EMBED_STACK_SIZE]; + */ + msgpack_unpack_struct(_stack) stack[MSGPACK_EMBED_STACK_SIZE]; }; @@ -69,17 +72,21 @@ msgpack_unpack_func(void, _init)(msgpack_unpack_struct(_context)* ctx) ctx->cs = CS_HEADER; ctx->trail = 0; ctx->top = 0; + /* ctx->stack = ctx->embed_stack; ctx->stack_size = MSGPACK_EMBED_STACK_SIZE; + */ ctx->stack[0].obj = msgpack_unpack_callback(_root)(&ctx->user); } +/* msgpack_unpack_func(void, _destroy)(msgpack_unpack_struct(_context)* ctx) { if(ctx->stack_size != MSGPACK_EMBED_STACK_SIZE) { free(ctx->stack); } } +*/ msgpack_unpack_func(msgpack_unpack_object, _data)(msgpack_unpack_struct(_context)* ctx) { @@ -99,7 +106,9 @@ msgpack_unpack_func(int, _execute)(msgpack_unpack_struct(_context)* ctx, const c unsigned int cs = ctx->cs; unsigned int top = ctx->top; msgpack_unpack_struct(_stack)* stack = ctx->stack; + /* unsigned int stack_size = ctx->stack_size; + */ msgpack_unpack_user* user = &ctx->user; msgpack_unpack_object obj; @@ -129,6 +138,7 @@ msgpack_unpack_func(int, _execute)(msgpack_unpack_struct(_context)* ctx, const c goto _fixed_trail_again #define start_container(func, count_, ct_) \ + if(top >= MSGPACK_EMBED_STACK_SIZE) { goto _failed; } /* FIXME */ \ if(msgpack_unpack_callback(func)(user, count_, &stack[top].obj) < 0) { goto _failed; } \ if((count_) == 0) { obj = stack[top].obj; goto _push; } \ stack[top].ct = ct_; \ @@ -136,6 +146,7 @@ msgpack_unpack_func(int, _execute)(msgpack_unpack_struct(_context)* ctx, const c ++top; \ /*printf("container %d count %d stack %d\n",stack[top].obj,count_,top);*/ \ /*printf("stack push %d\n", top);*/ \ + /* FIXME \ if(top >= stack_size) { \ if(stack_size == MSGPACK_EMBED_STACK_SIZE) { \ size_t csize = sizeof(msgpack_unpack_struct(_stack)) * MSGPACK_EMBED_STACK_SIZE; \ @@ -153,6 +164,7 @@ msgpack_unpack_func(int, _execute)(msgpack_unpack_struct(_context)* ctx, const c ctx->stack_size = stack_size = stack_size * 2; \ } \ } \ + */ \ goto _header_again #define NEXT_CS(p) \ diff --git a/perl/unpack.c b/perl/unpack.c index c520e028..69017f11 100644 --- a/perl/unpack.c +++ b/perl/unpack.c @@ -53,7 +53,6 @@ struct template_context; typedef struct template_context msgpack_unpack_t; static void template_init(msgpack_unpack_t* u); -static void template_destroy(msgpack_unpack_t* u); static SV* template_data(msgpack_unpack_t* u); @@ -143,7 +142,6 @@ SV* _msgpack_unpack(SV* data, int limit) { mp.user.source = &PL_sv_undef; obj = template_data(&mp); - template_destroy(&mp); if(ret < 0) { Perl_croak(aTHX_ "parse error."); @@ -316,7 +314,6 @@ XS(xs_unpacker_destroy) { } UNPACKER(ST(0), mp); - template_destroy(mp); Safefree(mp); XSRETURN(0);