From c9df91b04a429f9324afeefece28f21e7078e3ac Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 22 Jan 2011 01:03:02 -0800 Subject: upb bootstraps again! and with no memory leaks! --- core/upb_def.c | 40 +++++++++++++++++----------------------- core/upb_def.h | 2 +- core/upb_stream_vtbl.h | 1 + core/upb_string.c | 1 + tests/test_def.c | 4 +--- tests/test_string.c | 11 +++++++++++ 6 files changed, 32 insertions(+), 27 deletions(-) diff --git a/core/upb_def.c b/core/upb_def.c index a935930..c21843e 100644 --- a/core/upb_def.c +++ b/core/upb_def.c @@ -429,7 +429,7 @@ typedef struct _upb_unresolveddef { static upb_unresolveddef *upb_unresolveddef_new(upb_string *str) { upb_unresolveddef *def = malloc(sizeof(*def)); upb_def_init(&def->base, UPB_DEF_UNRESOLVED); - def->name = str; + def->name = upb_string_getref(str); return def; } @@ -445,6 +445,7 @@ static void upb_unresolveddef_free(struct _upb_unresolveddef *def) { static void upb_enumdef_free(upb_enumdef *e) { upb_enum_iter i; for(i = upb_enum_begin(e); !upb_enum_done(i); i = upb_enum_next(e, i)) { + // Frees the ref taken when the string was parsed. upb_string_unref(upb_enum_iter_name(i)); } upb_strtable_free(&e->ntoi); @@ -468,7 +469,7 @@ static upb_flow_t upb_enumdef_EnumValueDescriptorProto_value(void *_b, switch(f->number) { case GOOGLE_PROTOBUF_ENUMVALUEDESCRIPTORPROTO_NAME_FIELDNUM: upb_string_unref(b->name); - upb_string_getref(upb_value_getstr(val)); + b->name = upb_string_getref(upb_value_getstr(val)); b->saw_name = true; break; case GOOGLE_PROTOBUF_ENUMVALUEDESCRIPTORPROTO_NUMBER_FIELDNUM: @@ -495,6 +496,7 @@ static upb_flow_t upb_enumdef_EnumValueDescriptorProto_endmsg(void *_b) { // We don't unref "name" because we pass our ref to the iton entry of the // table. strtables can ref their keys, but the inttable doesn't know that // the value is a string. + b->name = NULL; return UPB_CONTINUE; } @@ -641,7 +643,7 @@ static upb_flow_t upb_fielddef_value(void *_b, upb_fielddef *f, upb_value val) { b->f->name = upb_string_getref(upb_value_getstr(val)); break; case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_NAME_FIELDNUM: { - if(b->f->def) upb_def_unref(b->f->def); + upb_def_unref(b->f->def); b->f->def = UPB_UPCAST(upb_unresolveddef_new(upb_value_getstr(val))); b->f->owned = true; break; @@ -847,6 +849,7 @@ static upb_symtab_ent *upb_resolve(upb_strtable *t, return e; } else { // Remove components from base until we find an entry or run out. + // TODO: This branch is totally broken, but currently not used. upb_string *sym_str = upb_string_new(); int baselen = upb_string_len(base); while(1) { @@ -1212,21 +1215,14 @@ static void upb_baredecoder_run(upb_src *src, upb_status *status) { upb_baredecoder *d = (upb_baredecoder*)src; assert(!upb_handlers_isempty(&d->dispatcher.top->handlers)); upb_string *str = NULL; - upb_strlen_t stack[UPB_MAX_NESTING]; + upb_strlen_t stack[UPB_MAX_NESTING] = {UPB_STRLEN_MAX}; upb_strlen_t *top = &stack[0]; - *top = upb_string_len(d->input); d->offset = 0; #define CHECK(x) if (x != UPB_CONTINUE && x != BEGIN_SUBMSG) goto err; CHECK(upb_dispatch_startmsg(&d->dispatcher)); while(d->offset < upb_string_len(d->input)) { - // Detect end-of-submessage. - while(d->offset >= *top) { - CHECK(upb_dispatch_endsubmsg(&d->dispatcher)); - d->offset = *(top--); - } - uint32_t key = upb_baredecoder_readv64(d); upb_fielddef f; f.number = key >> 3; @@ -1266,22 +1262,22 @@ static void upb_baredecoder_run(upb_src *src, upb_status *status) { } CHECK(upb_dispatch_value(&d->dispatcher, &f, v)); } + // Detect end-of-submessage. + while(d->offset >= *top) { + CHECK(upb_dispatch_endsubmsg(&d->dispatcher)); + d->offset = *(top--); + } } CHECK(upb_dispatch_endmsg(&d->dispatcher)); - printf("SUCCESS!!\n"); upb_string_unref(str); return; err: upb_copyerr(status, d->dispatcher.top->handlers.status); - upb_printerr(d->dispatcher.top->handlers.status); - upb_printerr(status); upb_string_unref(str); - printf("ERROR!!\n"); } -static upb_baredecoder *upb_baredecoder_new(upb_string *str) -{ +static upb_baredecoder *upb_baredecoder_new(upb_string *str) { static upb_src_vtbl vtbl = { &upb_baredecoder_sethandlers, &upb_baredecoder_run, @@ -1294,19 +1290,16 @@ static upb_baredecoder *upb_baredecoder_new(upb_string *str) return d; } -static void upb_baredecoder_free(upb_baredecoder *d) -{ +static void upb_baredecoder_free(upb_baredecoder *d) { upb_string_unref(d->input); free(d); } -static upb_src *upb_baredecoder_src(upb_baredecoder *d) -{ +static upb_src *upb_baredecoder_src(upb_baredecoder *d) { return &d->src; } -void upb_symtab_add_descriptorproto(upb_symtab *symtab) -{ +void upb_symtab_add_descriptorproto(upb_symtab *symtab) { // For the moment we silently decline to perform the operation if the symbols // already exist in the symtab. Revisit this when we have a better story // about whether syms in a table can be replaced. @@ -1329,4 +1322,5 @@ void upb_symtab_add_descriptorproto(upb_symtab *symtab) upb_symtab_unref(symtab); abort(); } + upb_status_uninit(&status); } diff --git a/core/upb_def.h b/core/upb_def.h index 9eb961a..d9bab97 100644 --- a/core/upb_def.h +++ b/core/upb_def.h @@ -77,7 +77,7 @@ INLINE void upb_def_ref(upb_def *def) { if(upb_atomic_ref(&def->refcount) && def->is_cyclic) _upb_def_cyclic_ref(def); } INLINE void upb_def_unref(upb_def *def) { - if(upb_atomic_unref(&def->refcount)) _upb_def_reftozero(def); + if(def && upb_atomic_unref(&def->refcount)) _upb_def_reftozero(def); } /* upb_fielddef ***************************************************************/ diff --git a/core/upb_stream_vtbl.h b/core/upb_stream_vtbl.h index e462122..fd71b2d 100644 --- a/core/upb_stream_vtbl.h +++ b/core/upb_stream_vtbl.h @@ -275,6 +275,7 @@ INLINE upb_flow_t upb_dispatch_endsubmsg(upb_dispatcher *d) { ret = d->top->handlers.set->endmsg(d->top->handlers.closure); if (ret != UPB_CONTINUE) return ret; --d->top; + assert(d->top >= d->stack); } return d->top->handlers.set->endsubmsg(d->top->handlers.closure); } diff --git a/core/upb_string.c b/core/upb_string.c index e9ff0d9..c599728 100644 --- a/core/upb_string.c +++ b/core/upb_string.c @@ -67,6 +67,7 @@ void upb_string_recycle(upb_string **_str) { str->ptr = NULL; upb_string_release(str); } else { + upb_string_unref(str); *_str = upb_string_new(); } } diff --git a/tests/test_def.c b/tests/test_def.c index 5be0672..2d2658f 100644 --- a/tests/test_def.c +++ b/tests/test_def.c @@ -10,13 +10,10 @@ int main() { int count; upb_def **defs = upb_symtab_getdefs(s, &count, UPB_DEF_ANY); for (int i = 0; i < count; i++) { - printf("Def with name: " UPB_STRFMT "\n", UPB_STRARG(defs[i]->fqname)); upb_def_unref(defs[i]); } free(defs); - printf("Size: %zd\n", sizeof(upb_ntof_ent)); - upb_string *str = upb_strdupc("google.protobuf.FileDescriptorSet"); upb_def *fds = upb_symtab_lookup(s, str); assert(fds != NULL); @@ -24,4 +21,5 @@ int main() { upb_def_unref(fds); upb_string_unref(str); upb_symtab_unref(s); + return 0; } diff --git a/tests/test_string.c b/tests/test_string.c index 6446806..ef0e2a9 100644 --- a/tests/test_string.c +++ b/tests/test_string.c @@ -40,6 +40,17 @@ static void test_dynamic() { upb_string_recycle(&str); assert(str != NULL); + // Take a ref and recycle; should create a new string and release a ref + // on the old one. + upb_string *strcp = upb_string_getref(str); + assert(strcp == str); + assert(upb_atomic_read(&str->refcount) == 2); + upb_string_recycle(&str); + assert(strcp != str); + assert(upb_atomic_read(&str->refcount) == 1); + assert(upb_atomic_read(&strcp->refcount) == 1); + upb_string_unref(strcp); + upb_strcpyc(str, static_str); assert(upb_string_len(str) == (sizeof(static_str) - 1)); const char *robuf = upb_string_getrobuf(str); -- cgit v1.2.3