From 3bd691a4975b2267ff04611507e766a7f9f87e83 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Fri, 8 May 2015 16:56:29 -0700 Subject: Google-internal development. --- upb/bindings/lua/upb.c | 29 +++++++++++++++++------------ upb/bindings/lua/upb/pb.c | 30 ++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 22 deletions(-) (limited to 'upb/bindings/lua') diff --git a/upb/bindings/lua/upb.c b/upb/bindings/lua/upb.c index 17fc0a8..5ad0235 100644 --- a/upb/bindings/lua/upb.c +++ b/upb/bindings/lua/upb.c @@ -139,7 +139,8 @@ bool lupb_openlib(lua_State *L, void *ptr, const char *name, // Pushes a new userdata with the given metatable and ensures that it has a // uservalue. -static void *newudata_with_userval(lua_State *L, size_t size, const char *type) { +static void *newudata_with_userval(lua_State *L, size_t size, + const char *type) { void *ret = lua_newuserdata(L, size); // Set metatable. @@ -952,17 +953,17 @@ static int lupb_msgdef_field(lua_State *L) { } static int lupb_msgiter_next(lua_State *L) { - upb_msg_iter *i = lua_touserdata(L, lua_upvalueindex(1)); - if (upb_msg_done(i)) return 0; + upb_msg_field_iter *i = lua_touserdata(L, lua_upvalueindex(1)); + if (upb_msg_field_done(i)) return 0; lupb_def_pushwrapper(L, UPB_UPCAST(upb_msg_iter_field(i)), NULL); - upb_msg_next(i); + upb_msg_field_next(i); return 1; } static int lupb_msgdef_fields(lua_State *L) { const upb_msgdef *m = lupb_msgdef_check(L, 1); - upb_msg_iter *i = lua_newuserdata(L, sizeof(upb_msg_iter)); - upb_msg_begin(i, m); + upb_msg_field_iter *i = lua_newuserdata(L, sizeof(upb_msg_field_iter)); + upb_msg_field_begin(i, m); // Need to guarantee that the msgdef outlives the iter. lua_pushvalue(L, 1); lua_pushcclosure(L, &lupb_msgiter_next, 2); @@ -1416,8 +1417,10 @@ static lupb_msgdef *lupb_msg_assignoffsets(lua_State *L, int narg) { size_t userval_idx = 1; // Assign offsets. - upb_msg_iter i; - for (upb_msg_begin(&i, lmd->md); !upb_msg_done(&i); upb_msg_next(&i)) { + upb_msg_field_iter i; + for (upb_msg_field_begin(&i, lmd->md); + !upb_msg_field_done(&i); + upb_msg_field_next(&i)) { upb_fielddef *f = upb_msg_iter_field(&i); if (in_userval(f)) { offsets[upb_fielddef_index(f)] = userval_idx++; @@ -1442,7 +1445,9 @@ static lupb_msgdef *lupb_msg_assignoffsets(lua_State *L, int narg) { lua_newtable(L); // This will be our userval. int idx = 1; - for (upb_msg_begin(&i, lmd->md); !upb_msg_done(&i); upb_msg_next(&i)) { + for (upb_msg_field_begin(&i, lmd->md); + !upb_msg_field_done(&i); + upb_msg_field_next(&i)) { upb_fielddef *f = upb_msg_iter_field(&i); if (upb_fielddef_type(f) == UPB_TYPE_MESSAGE) { bool created = lupb_def_pushwrapper(L, upb_fielddef_subdef(f), NULL); @@ -1660,9 +1665,9 @@ void callback(const void *closure, upb_handlers *h) { lua_State *L = (lua_State*)closure; lupb_def_pushwrapper(L, UPB_UPCAST(upb_handlers_msgdef(h)), NULL); lupb_msgdef *lmd = lupb_msg_assignoffsets(L, -1); - upb_msg_iter i; - upb_msg_begin(&i, upb_handlers_msgdef(h)); - for (; !upb_msg_done(&i); upb_msg_next(&i)) { + upb_msg_field_iter i; + upb_msg_field_begin(&i, upb_handlers_msgdef(h)); + for (; !upb_msg_field_done(&i); upb_msg_field_next(&i)) { upb_fielddef *f = upb_msg_iter_field(&i); int hasbit = upb_fielddef_index(f); uint16_t ofs = lmd->field_offsets[upb_fielddef_index(f)]; diff --git a/upb/bindings/lua/upb/pb.c b/upb/bindings/lua/upb/pb.c index c9f1f47..ea3d755 100644 --- a/upb/bindings/lua/upb/pb.c +++ b/upb/bindings/lua/upb/pb.c @@ -41,6 +41,13 @@ static int lupb_pbdecodermethod_new(lua_State *L) { return 1; // The DecoderMethod wrapper. } +// We implement upb's allocation function by allocating a Lua userdata. +// This is a raw hunk of memory that will be GC'd by Lua. +static void *lua_alloc(void *ud, size_t size) { + lua_State *L = ud; + return lua_newuserdata(L, size); +} + // Unlike most of our exposed Lua functions, this does not correspond to an // actual method on the underlying DecoderMethod. But it's convenient, and // important to implement in C because we can do stack allocation and @@ -61,19 +68,22 @@ static int lupb_pbdecodermethod_parse(lua_State *L) { // Handlers need this. lua_getuservalue(L, -1); - upb_pbdecoder decoder; upb_status status = UPB_STATUS_INIT; - upb_pbdecoder_init(&decoder, method, &status); + upb_env env; + upb_env_init(&env); + upb_env_reporterrorsto(&env, &status); upb_sink sink; upb_sink_reset(&sink, handlers, msg); - upb_pbdecoder_resetoutput(&decoder, &sink); - upb_bufsrc_putbuf(pb, len, upb_pbdecoder_input(&decoder)); - // TODO: Our need to call uninit isn't longjmp-safe; what if the decode - // triggers a Lua error? uninit is only needed if the decoder - // dynamically-allocated a growing stack -- ditch this feature and live with - // the compile-time limit? Or have a custom allocation function that - // allocates Lua GC-rooted memory? - upb_pbdecoder_uninit(&decoder); + upb_pbdecoder *decoder = upb_pbdecoder_create(&env, method, &sink); + upb_bufsrc_putbuf(pb, len, upb_pbdecoder_input(decoder)); + + // This won't get called in the error case, which longjmp's across us. But + // since we made our alloc function allocate only GC-able memory, that + // shouldn't matter. It *would* matter if the environment had references to + // any non-memory resources (ie. filehandles). As an alternative to this we + // could make the environment itself a userdata. + upb_env_uninit(&env); + lupb_checkstatus(L, &status); lua_pop(L, 1); // Uservalue. -- cgit v1.2.3 From 838009ba2b8ea1e99061c66e0fbd9cb53a96ec20 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Fri, 8 May 2015 17:20:55 -0700 Subject: Fixes for the open-source build. --- Makefile | 1 + upb/bindings/lua/upb.c | 2 +- upb/bindings/lua/upb/pb.c | 15 +++------------ upb/env.c | 8 +++++--- upb/env.h | 6 +++--- upb/pb/decoder.c | 4 +++- 6 files changed, 16 insertions(+), 20 deletions(-) (limited to 'upb/bindings/lua') diff --git a/Makefile b/Makefile index 5887fbe..e75fe54 100644 --- a/Makefile +++ b/Makefile @@ -131,6 +131,7 @@ make_objs_cc = $$(patsubst upb/$$(pc).cc,obj/upb/$$(pc).$(1),$$($$(call to_srcs, upb_SRCS = \ upb/def.c \ + upb/env.c \ upb/handlers.c \ upb/refcounted.c \ upb/shim/shim.c \ diff --git a/upb/bindings/lua/upb.c b/upb/bindings/lua/upb.c index 5ad0235..b35af24 100644 --- a/upb/bindings/lua/upb.c +++ b/upb/bindings/lua/upb.c @@ -1358,7 +1358,7 @@ static size_t align_up(size_t val, size_t align) { // If we always read/write as a consistent type to each value, this shouldn't // violate aliasing. -#define DEREF(msg, ofs, type) *(type*)(&msg->data[ofs]) +#define DEREF(msg, ofs, type) *(type*)((char*)msg + sizeof(lupb_msg) + ofs) lupb_msg *lupb_msg_check(lua_State *L, int narg) { lupb_msg *msg = luaL_checkudata(L, narg, LUPB_MSG); diff --git a/upb/bindings/lua/upb/pb.c b/upb/bindings/lua/upb/pb.c index ea3d755..920648f 100644 --- a/upb/bindings/lua/upb/pb.c +++ b/upb/bindings/lua/upb/pb.c @@ -41,13 +41,6 @@ static int lupb_pbdecodermethod_new(lua_State *L) { return 1; // The DecoderMethod wrapper. } -// We implement upb's allocation function by allocating a Lua userdata. -// This is a raw hunk of memory that will be GC'd by Lua. -static void *lua_alloc(void *ud, size_t size) { - lua_State *L = ud; - return lua_newuserdata(L, size); -} - // Unlike most of our exposed Lua functions, this does not correspond to an // actual method on the underlying DecoderMethod. But it's convenient, and // important to implement in C because we can do stack allocation and @@ -77,11 +70,9 @@ static int lupb_pbdecodermethod_parse(lua_State *L) { upb_pbdecoder *decoder = upb_pbdecoder_create(&env, method, &sink); upb_bufsrc_putbuf(pb, len, upb_pbdecoder_input(decoder)); - // This won't get called in the error case, which longjmp's across us. But - // since we made our alloc function allocate only GC-able memory, that - // shouldn't matter. It *would* matter if the environment had references to - // any non-memory resources (ie. filehandles). As an alternative to this we - // could make the environment itself a userdata. + // TODO: This won't get called in the error case, which longjmp's across us. + // This will cause the memory to leak. To remedy this, we should make the + // upb_env wrapped in a userdata that guarantees this will get called. upb_env_uninit(&env); lupb_checkstatus(L, &status); diff --git a/upb/env.c b/upb/env.c index ae5fb85..7fa3334 100644 --- a/upb/env.c +++ b/upb/env.c @@ -43,7 +43,7 @@ static void *default_alloc(void *_ud, void *ptr, size_t oldsize, size_t size) { UPB_UNUSED(oldsize); default_alloc_ud *ud = _ud; - mem_block *from = ptr ? ptr - sizeof(mem_block) : NULL; + mem_block *from = ptr ? (void*)((char*)ptr - sizeof(mem_block)) : NULL; #ifndef NDEBUG if (from) { @@ -214,7 +214,9 @@ UPB_FORCEINLINE static void *seeded_alloc(void *ud, void *ptr, size_t oldsize, upb_seededalloc *a = ud; size = align_up(size); - if (oldsize == 0 && size <= a->mem_limit - a->mem_ptr) { + assert(a->mem_limit >= a->mem_ptr); + + if (oldsize == 0 && size <= (size_t)(a->mem_limit - a->mem_ptr)) { // Fast path: we can satisfy from the initial allocation. void *ret = a->mem_ptr; a->mem_ptr += size; @@ -229,7 +231,7 @@ UPB_FORCEINLINE static void *seeded_alloc(void *ud, void *ptr, size_t oldsize, void upb_seededalloc_init(upb_seededalloc *a, void *mem, size_t len) { a->mem_base = mem; a->mem_ptr = mem; - a->mem_limit = mem + len; + a->mem_limit = (char*)mem + len; a->need_cleanup = false; a->returned_allocfunc = false; diff --git a/upb/env.h b/upb/env.h index 500b0ac..78dda20 100644 --- a/upb/env.h +++ b/upb/env.h @@ -177,9 +177,9 @@ UPB_DEFINE_STRUCT0(upb_seededalloc, void *default_alloc_ud; // Pointers for the initial memory region. - void *mem_base; - void *mem_ptr; - void *mem_limit; + char *mem_base; + char *mem_ptr; + char *mem_limit; // For future expansion, since the size of this struct is exposed to users. void *future1; diff --git a/upb/pb/decoder.c b/upb/pb/decoder.c index f23645d..a780666 100644 --- a/upb/pb/decoder.c +++ b/upb/pb/decoder.c @@ -966,7 +966,9 @@ size_t upb_pbdecoder_maxnesting(const upb_pbdecoder *d) { } bool upb_pbdecoder_setmaxnesting(upb_pbdecoder *d, size_t max) { - if (max < d->top - d->stack) { + assert(d->top >= d->stack); + + if (max < (size_t)(d->top - d->stack)) { // Can't set a limit smaller than what we are currently at. return false; } -- cgit v1.2.3