From ece08710a64e09baf4bdcb71752ce20ff1d4a757 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Fri, 1 Jan 2010 17:31:14 -0800 Subject: Bugfixes: descriptorgen works without leaks! --- src/upb_atomic.h | 2 +- src/upb_data.c | 25 ++++++++++++++----------- src/upb_data.h | 4 ++-- src/upb_def.c | 29 +++++++++++++++++------------ 4 files changed, 34 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/upb_atomic.h b/src/upb_atomic.h index de2238c..c2cb8ba 100644 --- a/src/upb_atomic.h +++ b/src/upb_atomic.h @@ -59,7 +59,7 @@ INLINE bool upb_atomic_add(upb_atomic_refcount_t *a, int val) { return a->v == 0; } -INLINE bool upb_atomic_fetch_and_add(upb_atomic_refcount_t *a, int val) { +INLINE int upb_atomic_fetch_and_add(upb_atomic_refcount_t *a, int val) { int ret = a->v; a->v += val; return ret; diff --git a/src/upb_data.c b/src/upb_data.c index 8de0c62..e1aebdf 100644 --- a/src/upb_data.c +++ b/src/upb_data.c @@ -24,19 +24,21 @@ static uint32_t round_up_to_pow2(uint32_t v) /* upb_data *******************************************************************/ -static void data_elem_unref(void *d, struct upb_fielddef *f) { - if(f->type == UPB_TYPE(MESSAGE) || f->type == UPB_TYPE(GROUP)) { - upb_msg_unref((upb_msg*)d, upb_downcast_msgdef(f->def)); - } else if(f->type == UPB_TYPE(STRING) || f->type == UPB_TYPE(BYTES)) { - upb_string_unref((upb_string*)d); +static void data_elem_unref(union upb_value_ptr p, struct upb_fielddef *f) { + if(upb_issubmsg(f)) { + upb_msg_unref(*p.msg, upb_downcast_msgdef(f->def)); + } else if(upb_isstring(f)) { + upb_string_unref(*p.str); + } else { + assert(false); } } -static void data_unref(void *d, struct upb_fielddef *f) { +static void data_unref(union upb_value_ptr p, struct upb_fielddef *f) { if(upb_isarray(f)) { - upb_array_unref((upb_array*)d, f); + upb_array_unref(*p.arr, f); } else { - data_elem_unref(d, f); + data_elem_unref(p, f); } } @@ -179,7 +181,8 @@ void _upb_array_free(upb_array *a, struct upb_fielddef *f) if(upb_elem_ismm(f)) { for(upb_arraylen_t i = 0; i < a->refcounted.size; i++) { union upb_value_ptr p = _upb_array_getptr(a, f, i); - data_elem_unref(p._void, f); + if(!*p.data) continue; + data_elem_unref(p, f); } } if(a->refcounted.size != 0) free(a->common.elements._void); @@ -241,8 +244,8 @@ void _upb_msg_free(upb_msg *msg, struct upb_msgdef *md) for(int i = 0; i < md->num_fields; i++) { struct upb_fielddef *f = &md->fields[i]; union upb_value_ptr p = _upb_msg_getptr(msg, f); - if(!upb_field_ismm(f) || !p._void) continue; - data_unref(p._void, f); + if(!upb_field_ismm(f) || !*p.data) continue; + data_unref(p, f); } upb_def_unref(UPB_UPCAST(md)); free(msg); diff --git a/src/upb_data.h b/src/upb_data.h index 2a7940e..6dc343b 100644 --- a/src/upb_data.h +++ b/src/upb_data.h @@ -374,14 +374,14 @@ void _upb_array_free(upb_array *a, struct upb_fielddef *f); INLINE union upb_value_ptr _upb_array_getptr(upb_array *a, struct upb_fielddef *f, upb_arraylen_t elem) { - assert(elem < upb_array_len(a)); size_t type_size = upb_type_info[f->type].size; union upb_value_ptr p = {._void = &a->common.elements.uint8[elem * type_size]}; return p; } INLINE union upb_value upb_array_get(upb_array *a, struct upb_fielddef *f, - int elem) { + upb_arraylen_t elem) { + assert(elem < upb_array_len(a)); return upb_value_read(_upb_array_getptr(a, f, elem), f->type); } diff --git a/src/upb_def.c b/src/upb_def.c index f824910..b1c4ab2 100644 --- a/src/upb_def.c +++ b/src/upb_def.c @@ -216,6 +216,11 @@ static void fielddef_uninit(struct upb_fielddef *f) } } +static void fielddef_free(struct upb_fielddef *f) { + fielddef_uninit(f); + free(f); +} + static void fielddef_copy(struct upb_fielddef *dst, struct upb_fielddef *src) { *dst = *src; @@ -501,9 +506,10 @@ static void insert_enum(struct upb_strtable *t, if(!fqname) return; struct symtab_ent e; - e.e.key = fqname; // Donating our ref to the table. + e.e.key = fqname; e.def = UPB_UPCAST(enumdef_new(ed, fqname)); upb_strtable_insert(t, &e.e); + upb_string_unref(fqname); } static void insert_message(struct upb_strtable *t, @@ -518,17 +524,20 @@ static void insert_message(struct upb_strtable *t, int num_fields = d->set_flags.has.field ? d->field->len : 0; struct symtab_ent e; e.e.key = fqname; + + // Gather our list of fields, sorting if necessary. struct upb_fielddef **fielddefs = malloc(sizeof(*fielddefs) * num_fields); for (int i = 0; i < num_fields; i++) { google_protobuf_FieldDescriptorProto *fd = d->field->elements[i]; fielddefs[i] = fielddef_new(fd); } if(sort) fielddef_sort(fielddefs, d->field->len); + + // Create the msgdef with that list of fields. e.def = UPB_UPCAST(msgdef_new(fielddefs, d->field->len, fqname, status)); - for (int i = 0; i < num_fields; i++) { - fielddef_uninit(fielddefs[i]); - free(fielddefs[i]); - } + + // Cleanup. + for (int i = 0; i < num_fields; i++) fielddef_free(fielddefs[i]); free(fielddefs); if(!upb_ok(status)) goto error; @@ -545,6 +554,7 @@ static void insert_message(struct upb_strtable *t, insert_enum(t, d->enum_type->elements[i], fqname, status); error: + // Free the ref we got from try_define(). upb_string_unref(fqname); } @@ -601,15 +611,10 @@ static void addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs, struct upb_status *status) { upb_string *pkg; - // Temporary hack until the static data is integrated into our - // memory-management scheme. - bool should_unref; if(fd->set_flags.has.package) { - pkg = fd->package; - should_unref = false; + pkg = upb_string_getref(fd->package, UPB_REF_FROZEN); } else { pkg = upb_string_new(); - should_unref = true; } if(fd->set_flags.has.message_type) @@ -620,7 +625,7 @@ static void addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs, for(unsigned int i = 0; i < fd->enum_type->len; i++) insert_enum(addto, fd->enum_type->elements[i], pkg, status); - if(should_unref) upb_string_unref(pkg); + upb_string_unref(pkg); if(!upb_ok(status)) { // TODO: make sure we don't leak any memory in this case. -- cgit v1.2.3