From ae0beee2854b977f472d48cd149b880b074b59c5 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 10 Jul 2010 19:37:47 -0700 Subject: Fixed upb_string error with strange vsnprintf() behavior. --- core/upb.c | 9 +++++++++ core/upb.h | 1 + core/upb_def.c | 49 +++++++++++++++++++++++++++++++++++++------------ core/upb_string.c | 13 +++++++++---- tests/test_string.c | 9 +++++++++ 5 files changed, 65 insertions(+), 16 deletions(-) diff --git a/core/upb.c b/core/upb.c index 9ed5617..d581bbe 100644 --- a/core/upb.c +++ b/core/upb.c @@ -64,3 +64,12 @@ void upb_status_reset(upb_status *status) { upb_string_unref(status->str); status->str = NULL; } + +void upb_printerr(upb_status *status) { + if(status->str) { + fprintf(stderr, "code: %d, msg: " UPB_STRFMT "\n", + status->code, UPB_STRARG(status->str)); + } else { + fprintf(stderr, "code: %d, no msg\n", status->code); + } +} diff --git a/core/upb.h b/core/upb.h index 630d9e1..13317bb 100644 --- a/core/upb.h +++ b/core/upb.h @@ -200,6 +200,7 @@ INLINE void upb_status_init(upb_status *status) { status->str = NULL; } +void upb_printerr(upb_status *status); void upb_status_reset(upb_status *status); void upb_seterr(upb_status *status, enum upb_status_code code, const char *msg, ...); diff --git a/core/upb_def.c b/core/upb_def.c index 0f48559..2b2916e 100644 --- a/core/upb_def.c +++ b/core/upb_def.c @@ -21,7 +21,7 @@ typedef struct { static void upb_deflist_init(upb_deflist *l) { l->size = 8; - l->defs = malloc(l->size); + l->defs = malloc(l->size * sizeof(void*)); l->len = 0; } @@ -34,7 +34,7 @@ static void upb_deflist_uninit(upb_deflist *l) { static void upb_deflist_push(upb_deflist *l, upb_def *d) { if(l->len == l->size) { l->size *= 2; - l->defs = realloc(l->defs, l->size); + l->defs = realloc(l->defs, l->size * sizeof(void*)); } l->defs[l->len++] = d; } @@ -238,6 +238,7 @@ static void upb_enumdef_free(upb_enumdef *e) { free(e); } +// google.protobuf.EnumValueDescriptorProto. static bool upb_addenum_val(upb_src *src, upb_enumdef *e, upb_status *status) { int32_t number = -1; @@ -245,13 +246,13 @@ static bool upb_addenum_val(upb_src *src, upb_enumdef *e, upb_status *status) upb_fielddef *f; while((f = upb_src_getdef(src)) != NULL) { switch(f->number) { - case GOOGLE_PROTOBUF_ENUMVALUEDESCRIPTORPROTO_NUMBER_FIELDNUM: - CHECKSRC(upb_src_getint32(src, &number)); - break; case GOOGLE_PROTOBUF_ENUMVALUEDESCRIPTORPROTO_NAME_FIELDNUM: name = upb_string_tryrecycle(name); CHECKSRC(upb_src_getstr(src, name)); break; + case GOOGLE_PROTOBUF_ENUMVALUEDESCRIPTORPROTO_NUMBER_FIELDNUM: + CHECKSRC(upb_src_getint32(src, &number)); + break; default: CHECKSRC(upb_src_skipval(src)); break; @@ -278,6 +279,7 @@ err: return false; } +// google.protobuf.EnumDescriptorProto. static bool upb_addenum(upb_src *src, upb_deflist *defs, upb_status *status) { upb_enumdef *e = malloc(sizeof(*e)); @@ -290,8 +292,11 @@ static bool upb_addenum(upb_src *src, upb_deflist *defs, upb_status *status) case GOOGLE_PROTOBUF_ENUMDESCRIPTORPROTO_NAME_FIELDNUM: e->base.fqname = upb_string_tryrecycle(e->base.fqname); CHECKSRC(upb_src_getstr(src, e->base.fqname)); + break; case GOOGLE_PROTOBUF_ENUMDESCRIPTORPROTO_VALUE_FIELDNUM: + CHECKSRC(upb_src_startmsg(src)); CHECK(upb_addenum_val(src, e, status)); + CHECKSRC(upb_src_endmsg(src)); break; default: upb_src_skipval(src); @@ -729,8 +734,10 @@ err: // We need to free all defs from "tmptab." upb_rwlock_unlock(&s->lock); for(upb_symtab_ent *e = upb_strtable_begin(&tmptab); e; - e = upb_strtable_next(&tmptab, &e->e)) + e = upb_strtable_next(&tmptab, &e->e)) { + fprintf(stderr, "Unreffing def: '" UPB_STRFMT "'\n", UPB_STRARG(e->e.key)); upb_def_unref(e->def); + } upb_strtable_free(&tmptab); return false; } @@ -914,10 +921,12 @@ static upb_fielddef *upb_baredecoder_getdef(upb_baredecoder *d) key = upb_baredecoder_readv32(d); d->wire_type = key & 0x7; d->field.number = key >> 3; + fprintf(stderr, "field num: %d, wire_type: %d\n", d->field.number, d->wire_type); if(d->wire_type == UPB_WIRE_TYPE_DELIMITED) { // For delimited wire values we parse the length now, since we need it in // all cases. d->delimited_len = upb_baredecoder_readv32(d); + fprintf(stderr, "delimited size: %d\n", d->delimited_len); } return &d->field; } @@ -944,6 +953,7 @@ static bool upb_baredecoder_getval(upb_baredecoder *d, upb_valueptr val) *val.uint32 = upb_baredecoder_readf32(d); break; default: + *(char*)0 = 0; assert(false); } return true; @@ -951,19 +961,24 @@ static bool upb_baredecoder_getval(upb_baredecoder *d, upb_valueptr val) static bool upb_baredecoder_skipval(upb_baredecoder *d) { - upb_value val; - return upb_baredecoder_getval(d, upb_value_addrof(&val)); + if(d->wire_type == UPB_WIRE_TYPE_DELIMITED) { + d->offset += d->delimited_len; + return true; + } else { + upb_value val; + return upb_baredecoder_getval(d, upb_value_addrof(&val)); + } } static bool upb_baredecoder_startmsg(upb_baredecoder *d) { - *(d->top++) = d->offset + d->delimited_len; + *(++d->top) = d->offset + d->delimited_len; return true; } static bool upb_baredecoder_endmsg(upb_baredecoder *d) { - d->offset = *(--d->top); + d->offset = *(d->top--); return true; } @@ -980,7 +995,9 @@ static upb_baredecoder *upb_baredecoder_new(upb_string *str) { upb_baredecoder *d = malloc(sizeof(*d)); d->input = upb_string_getref(str); + d->offset = 0; d->top = &d->stack[0]; + *(d->top) = upb_string_len(d->input); upb_src_init(&d->src, &upb_baredecoder_src_vtbl); return d; } @@ -1001,9 +1018,17 @@ void upb_symtab_add_descriptorproto(upb_symtab *symtab) // TODO: allow upb_strings to be static or on the stack. upb_string *descriptor = upb_strduplen(descriptor_pb, descriptor_pb_len); upb_baredecoder *decoder = upb_baredecoder_new(descriptor); - upb_status status; + upb_status status = UPB_STATUS_INIT; upb_symtab_addfds(symtab, upb_baredecoder_src(decoder), &status); - assert(upb_ok(&status)); upb_baredecoder_free(decoder); upb_string_unref(descriptor); + + if(!upb_ok(&status)) { + // upb itself is corrupt. + upb_printerr(&status); + upb_symtab_unref(symtab); + abort(); + } + fprintf(stderr, "Claims to have succeeded\n"); + upb_printerr(&status); } diff --git a/core/upb_string.c b/core/upb_string.c index 2f487aa..3563c9e 100644 --- a/core/upb_string.c +++ b/core/upb_string.c @@ -87,6 +87,7 @@ char *upb_string_getrwbuf(upb_string *str, upb_strlen_t len) { void upb_string_substr(upb_string *str, upb_string *target_str, upb_strlen_t start, upb_strlen_t len) { + if(str->ptr) *(char*)0 = 0; assert(str->ptr == NULL); str->src = upb_string_getref(target_str); str->ptr = upb_string_getrobuf(target_str) + start; @@ -103,11 +104,15 @@ void upb_string_vprintf(upb_string *str, const char *format, va_list args) { uint32_t true_size = vsnprintf(buf, size, format, args_copy); va_end(args_copy); - if (true_size > size) { - // Need to reallocate. + if (true_size >= size) { + // Need to reallocate. We reallocate even if the sizes were equal, + // because snprintf excludes the terminating NULL from its count. + // We don't care about the terminating NULL, but snprintf might + // bail out of printing even other characters if it doesn't have + // enough space to write the NULL also. str = upb_string_tryrecycle(str); - buf = upb_string_getrwbuf(str, true_size); - vsnprintf(buf, true_size, format, args); + buf = upb_string_getrwbuf(str, true_size + 1); + vsnprintf(buf, true_size + 1, format, args); } str->len = true_size; } diff --git a/tests/test_string.c b/tests/test_string.c index 5869b70..46f35b9 100644 --- a/tests/test_string.c +++ b/tests/test_string.c @@ -32,6 +32,7 @@ int main() { // Make string alias part of another string. str2 = upb_strdupc("WXYZ"); + str = upb_string_tryrecycle(str); upb_string_substr(str, str2, 1, 2); assert(upb_string_len(str) == 2); assert(upb_string_len(str2) == 4); @@ -63,9 +64,17 @@ int main() { // Test printf. str = upb_string_tryrecycle(str); upb_string_printf(str, "Number: %d, String: %s", 5, "YO!"); + assert(upb_streqlc(str, "Number: 5, String: YO!")); + + // Test asprintf + upb_string *str3 = upb_string_asprintf("Yo %s: " UPB_STRFMT "\n", + "Josh", UPB_STRARG(str)); + const char expected[] = "Yo Josh: Number: 5, String: YO!\n"; + assert(upb_streqlc(str3, expected)); upb_string_unref(str); upb_string_unref(str2); + upb_string_unref(str3); // Unref of NULL is harmless. upb_string_unref(NULL); -- cgit v1.2.3