From 4667ed4be921b2142321e47c8ccc6a35a9189277 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sun, 6 Feb 2011 08:21:47 -0800 Subject: All tests pass again, valgrind-clean! Next up: benchmarks. --- core/upb_def.c | 40 ++++++++++++++++++++++++++++++---------- core/upb_def.h | 24 ++++++++++++++++++------ core/upb_glue.c | 16 ++++++++++++++++ core/upb_glue.h | 6 +++++- core/upb_msg.h | 4 ++-- 5 files changed, 71 insertions(+), 19 deletions(-) (limited to 'core') diff --git a/core/upb_def.c b/core/upb_def.c index 298ede1..8abb8bc 100644 --- a/core/upb_def.c +++ b/core/upb_def.c @@ -5,10 +5,13 @@ */ #include +#include #include "descriptor_const.h" #include "descriptor.h" #include "upb_def.h" +#define alignof(t) offsetof(struct { char c; t x; }, x) + /* Rounds p up to the next multiple of t. */ static size_t upb_align_up(size_t val, size_t align) { return val % align == 0 ? val : val + align - (val % align); @@ -24,7 +27,7 @@ static int upb_div_round_up(int numerator, int denominator) { * join("", "Baz") -> "Baz" * Caller owns a ref on the returned string. */ static upb_string *upb_join(upb_string *base, upb_string *name) { - if (upb_string_len(base) == 0) { + if (!base || upb_string_len(base) == 0) { return upb_string_getref(name); } else { return upb_string_asprintf(UPB_STRFMT "." UPB_STRFMT, @@ -725,14 +728,22 @@ static upb_flow_t upb_msgdef_endmsg(void *_b) { // together. f->field_index = i; + size_t size, align; + if (upb_isarray(f)) { + size = sizeof(void*); + align = alignof(void*); + } else { + size = type_info->size; + align = type_info->align; + } // General alignment rules are: each member must be at an address that is a // multiple of that type's alignment. Also, the size of the structure as a // whole must be a multiple of the greatest alignment of any member. - size_t offset = upb_align_up(m->size, type_info->align); + size_t offset = upb_align_up(m->size, align); // Offsets are relative to the end of the refcount. f->byte_offset = offset - sizeof(upb_atomic_refcount_t); - m->size = offset + type_info->size; - max_align = UPB_MAX(max_align, type_info->align); + m->size = offset + size; + max_align = UPB_MAX(max_align, align); } free(sorted_fields); @@ -1054,6 +1065,7 @@ upb_symtab *upb_symtab_new() upb_atomic_refcount_init(&s->refcount, 1); upb_rwlock_init(&s->lock); upb_strtable_init(&s->symtab, 16, sizeof(upb_symtab_ent)); + s->fds_msgdef = NULL; return s; } @@ -1304,12 +1316,7 @@ 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. - upb_def *def = upb_symtab_lookup( - symtab, UPB_STRLIT("google.protobuf.FileDescriptorSet")); - if(def) { - upb_def_unref(def); - return; - } + if(symtab->fds_msgdef) upb_def_unref(UPB_UPCAST(symtab->fds_msgdef)); upb_baredecoder *decoder = upb_baredecoder_new(&descriptor_str); upb_status status = UPB_STATUS_INIT; @@ -1323,5 +1330,18 @@ void upb_symtab_add_descriptorproto(upb_symtab *symtab) { upb_symtab_unref(symtab); abort(); } + upb_def *def = upb_symtab_lookup( + symtab, UPB_STRLIT("google.protobuf.FileDescriptorSet")); + if (!def || (symtab->fds_msgdef = upb_dyncast_msgdef(def)) == NULL) { + // upb itself is corrupt. + abort(); + } + upb_def_unref(def); // The symtab already holds a ref on it. upb_status_uninit(&status); } + +upb_msgdef *upb_symtab_fds_def(upb_symtab *s) { + assert(s->fds_msgdef != NULL); + upb_def_ref(UPB_UPCAST(s->fds_msgdef)); + return s->fds_msgdef; +} diff --git a/core/upb_def.h b/core/upb_def.h index e95aec3..34f31ec 100644 --- a/core/upb_def.h +++ b/core/upb_def.h @@ -254,11 +254,13 @@ INLINE int32_t upb_enum_iter_number(upb_enum_iter iter) { // A SymbolTable is where upb_defs live. It is empty when first constructed. // Clients add definitions to the symtab by supplying unserialized or // serialized descriptors (as defined in descriptor.proto). -typedef struct { +struct _upb_symtab { upb_atomic_refcount_t refcount; upb_rwlock_t lock; // Protects all members except the refcount. upb_strtable symtab; // The symbol table. -} upb_symtab; + upb_msgdef *fds_msgdef; // Msgdef for google.protobuf.FileDescriptorSet. +}; +typedef struct _upb_symtab upb_symtab; // Initializes a upb_symtab. Contexts are not freed explicitly, but unref'd // when the caller is done with them. @@ -293,10 +295,13 @@ upb_def *upb_symtab_lookup(upb_symtab *s, upb_string *sym); upb_def **upb_symtab_getdefs(upb_symtab *s, int *count, upb_deftype_t type); // "fds" is a upb_src that will yield data from the -// google.protobuf.FileDescriptorSet message type. upb_symtab_addfds() adds -// all the definitions from the given FileDescriptorSet and adds them to the -// symtab. status indicates whether the operation was successful or not, and -// the error message (if any). +// google.protobuf.FileDescriptorSet message type. It is not necessary that +// the upb_def for FileDescriptorSet came from this symtab, but it must be +// compatible with the official descriptor.proto, as published by Google. +// +// upb_symtab_addfds() adds all the definitions from the given +// FileDescriptorSet and adds them to the symtab. status indicates whether the +// operation was successful or not, and the error message (if any). // // TODO: should this allow redefinition? Either is possible, but which is // more useful? Maybe it should be an option. @@ -307,6 +312,13 @@ void upb_symtab_addfds(upb_symtab *s, upb_src *desc, upb_status *status); // specify other defs and allow them to be loaded. void upb_symtab_add_descriptorproto(upb_symtab *s); +// Returns the upb_msgdef for google.protobuf.FileDescriptorSet, which the +// caller owns a ref on. This is a convenience method that is equivalent to +// looking up the symbol called "google.protobuf.FileDescriptorSet" yourself, +// except that it only will return a def that was added by +// upb_symtab_add_descriptorproto(). +upb_msgdef *upb_symtab_fds_def(upb_symtab *s); + /* upb_def casts **************************************************************/ diff --git a/core/upb_glue.c b/core/upb_glue.c index 7540a3a..541827e 100644 --- a/core/upb_glue.c +++ b/core/upb_glue.c @@ -36,3 +36,19 @@ void upb_strtomsg(upb_string *str, upb_msg *msg, upb_msgdef *md, upb_msgpopulator_uninit(&p); upb_handlers_uninit(&h); } + +void upb_parsedesc(upb_symtab *symtab, upb_string *str, upb_status *status) { + upb_stringsrc strsrc; + upb_stringsrc_init(&strsrc); + upb_stringsrc_reset(&strsrc, str); + + upb_decoder d; + upb_msgdef *fds_msgdef = upb_symtab_fds_def(symtab); + upb_decoder_init(&d, fds_msgdef); + upb_decoder_reset(&d, upb_stringsrc_bytesrc(&strsrc)); + + upb_symtab_addfds(symtab, upb_decoder_src(&d), status); + upb_stringsrc_uninit(&strsrc); + upb_decoder_uninit(&d); + upb_def_unref(UPB_UPCAST(fds_msgdef)); +} diff --git a/core/upb_glue.h b/core/upb_glue.h index 61111f5..ca32436 100644 --- a/core/upb_glue.h +++ b/core/upb_glue.h @@ -25,16 +25,20 @@ extern "C" { // Forward-declares so we don't have to include everything in this .h file. // Clients should use the regular, typedef'd names (eg. upb_string). -struct _upb_string; struct _upb_msg; struct _upb_msgdef; struct _upb_status; +struct _upb_string; +struct _upb_symtab; // Decodes the given string, which must be in protobuf binary format, to the // given upb_msg with msgdef "md", storing the status of the operation in "s". void upb_strtomsg(struct _upb_string *str, struct _upb_msg *msg, struct _upb_msgdef *md, struct _upb_status *s); +void upb_parsedesc(struct _upb_symtab *symtab, struct _upb_string *str, + struct _upb_status *status); + #ifdef __cplusplus } /* extern "C" */ #endif diff --git a/core/upb_msg.h b/core/upb_msg.h index 475b346..1318e08 100644 --- a/core/upb_msg.h +++ b/core/upb_msg.h @@ -88,7 +88,7 @@ INLINE upb_value upb_value_read(upb_valueptr ptr, upb_fieldtype_t ft) { val.type = UPB_VALUETYPE_ARRAY; #endif break; - default: printf("type: %d\n", ft); assert(false); + default: assert(false); } return val; @@ -202,7 +202,7 @@ INLINE bool upb_msg_has(upb_msg *msg, upb_fielddef *f) { } INLINE upb_value upb_msg_get(upb_msg *msg, upb_fielddef *f) { - return upb_value_read(_upb_msg_getptr(msg, f), f->type); + return upb_value_read(_upb_msg_getptr(msg, f), upb_field_valuetype(f)); } // Unsets all field values back to their defaults. -- cgit v1.2.3