From f9afc3e55bbc289df41606d493377318c6645817 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 13 Jan 2016 18:04:48 -0800 Subject: Changed JSON parser/printer to correctly camelCase names. --- upb/def.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'upb/def.h') diff --git a/upb/def.h b/upb/def.h index 2c29cb5..d19ab18 100644 --- a/upb/def.h +++ b/upb/def.h @@ -307,6 +307,12 @@ class upb::FieldDef { uint32_t number() const; /* Returns 0 if uninitialized. */ bool is_extension() const; + /* Get the JSON name for this field. This will copy the JSON name into the + * given buffer, which must have size of at least "strlen(name()) + 1". + * The string will be NULL-terminated. Returns false if uninitialized. + */ + bool GetJsonName(char* buf) const; + /* For UPB_TYPE_MESSAGE fields only where is_tag_delimited() == false, * indicates whether this field should have lazy parsing handlers that yield * the unparsed string for the submessage. @@ -472,6 +478,16 @@ class upb::FieldDef { bool set_name(const char* name, upb::Status* s); bool set_name(const std::string& name, upb::Status* s); + /* Sets the JSON name to the given string. */ + /* TODO(haberman): implement. Right now only default json_name (camelCase) + * is supported. */ + bool set_json_name(const char* json_name, upb::Status* s); + bool set_json_name(const std::string& name, upb::Status* s); + + /* Clears the JSON name. This will make it revert to its default, which is + * a camelCased version of the regular field name. */ + void clear_json_name(); + void set_integer_format(IntegerFormat format); bool set_tag_delimited(bool tag_delimited, upb::Status* s); @@ -536,6 +552,7 @@ const char *upb_fielddef_name(const upb_fielddef *f); bool upb_fielddef_isextension(const upb_fielddef *f); bool upb_fielddef_lazy(const upb_fielddef *f); bool upb_fielddef_packed(const upb_fielddef *f); +bool upb_fielddef_getjsonname(const upb_fielddef *f, char *buf); const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f); const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f); upb_msgdef *upb_fielddef_containingtype_mutable(upb_fielddef *f); @@ -570,6 +587,8 @@ void upb_fielddef_setdescriptortype(upb_fielddef *f, int type); void upb_fielddef_setlabel(upb_fielddef *f, upb_label_t label); bool upb_fielddef_setnumber(upb_fielddef *f, uint32_t number, upb_status *s); bool upb_fielddef_setname(upb_fielddef *f, const char *name, upb_status *s); +bool upb_fielddef_setjsonname(upb_fielddef *f, const char *name, upb_status *s); +bool upb_fielddef_clearjsonname(upb_fielddef *f); bool upb_fielddef_setcontainingtypename(upb_fielddef *f, const char *name, upb_status *s); void upb_fielddef_setisextension(upb_fielddef *f, bool is_extension); @@ -1346,6 +1365,15 @@ inline bool FieldDef::set_name(const char *name, Status* s) { inline bool FieldDef::set_name(const std::string& name, Status* s) { return upb_fielddef_setname(this, upb_safecstr(name), s); } +inline bool FieldDef::set_json_name(const char *name, Status* s) { + return upb_fielddef_setjsonname(this, name, s); +} +inline bool FieldDef::set_json_name(const std::string& name, Status* s) { + return upb_fielddef_setjsonname(this, upb_safecstr(name), s); +} +inline void FieldDef::clear_json_name() { + upb_fielddef_clearjsonname(this); +} inline bool FieldDef::set_containing_type_name(const char *name, Status* s) { return upb_fielddef_setcontainingtypename(this, name, s); } -- cgit v1.2.3 From 458da2563fa674bbcfd32ade2254b3915f751491 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Tue, 16 Feb 2016 23:13:53 -0800 Subject: Addressed code review comments. --- upb/def.c | 28 +++++++++++++++++++--------- upb/def.h | 28 +++++++++++++++++++++++----- upb/json/parser.c | 18 +++++++++++++----- upb/json/parser.rl | 18 +++++++++++++----- upb/json/printer.c | 7 ++++--- 5 files changed, 72 insertions(+), 27 deletions(-) (limited to 'upb/def.h') diff --git a/upb/def.c b/upb/def.c index a1dc192..abb5b05 100644 --- a/upb/def.c +++ b/upb/def.c @@ -722,33 +722,43 @@ const char *upb_fielddef_name(const upb_fielddef *f) { return upb_def_fullname(upb_fielddef_upcast(f)); } -bool upb_fielddef_getjsonname(const upb_fielddef *f, char *buf) { +size_t upb_fielddef_getjsonname(const upb_fielddef *f, char *buf, size_t len) { const char *name = upb_fielddef_name(f); - size_t i, j; + size_t src, dst = 0; bool ucase_next = false; - if (!name) return false; +#define WRITE(byte) \ + ++dst; \ + if (dst < len) buf[dst - 1] = byte; \ + else if (dst == len) buf[dst - 1] = '\0' + + if (!name) { + WRITE('\0'); + return 0; + } /* Implement the transformation as described in the spec: * 1. upper case all letters after an underscore. * 2. remove all underscores. */ - for (i = 0, j = 0; name[i]; i++) { - if (name[i] == '_') { + for (src = 0; name[src]; src++) { + if (name[src] == '_') { ucase_next = true; continue; } if (ucase_next) { - buf[j++] = toupper(name[i]); + WRITE(toupper(name[src])); ucase_next = false; } else { - buf[j++] = name[i]; + WRITE(name[src]); } } - buf[j] = '\0'; - return true; + WRITE('\0'); + return dst; + +#undef WRITE } const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f) { diff --git a/upb/def.h b/upb/def.h index d19ab18..8365574 100644 --- a/upb/def.h +++ b/upb/def.h @@ -307,11 +307,26 @@ class upb::FieldDef { uint32_t number() const; /* Returns 0 if uninitialized. */ bool is_extension() const; - /* Get the JSON name for this field. This will copy the JSON name into the - * given buffer, which must have size of at least "strlen(name()) + 1". - * The string will be NULL-terminated. Returns false if uninitialized. + /* Copies the JSON name for this field into the given buffer. Returns the + * actual size of the JSON name, including the NULL terminator. If the + * return value is 0, the JSON name is unset. If the return value is + * greater than len, the JSON name was truncated. The buffer is always + * NULL-terminated if len > 0. + * + * The JSON name always defaults to a camelCased version of the regular + * name. However if the regular name is unset, the JSON name will be unset + * also. */ - bool GetJsonName(char* buf) const; + size_t GetJsonName(char* buf, size_t len) const; + + /* Convenience version of the above function which copies the JSON name + * into the given string, returning false if the name is not set. */ + template + bool GetJsonName(T* str) { + str->resize(GetJsonName(NULL, 0)); + GetJsonName(&(*str)[0], str->size()); + return str->size() > 0; + } /* For UPB_TYPE_MESSAGE fields only where is_tag_delimited() == false, * indicates whether this field should have lazy parsing handlers that yield @@ -552,7 +567,7 @@ const char *upb_fielddef_name(const upb_fielddef *f); bool upb_fielddef_isextension(const upb_fielddef *f); bool upb_fielddef_lazy(const upb_fielddef *f); bool upb_fielddef_packed(const upb_fielddef *f); -bool upb_fielddef_getjsonname(const upb_fielddef *f, char *buf); +size_t upb_fielddef_getjsonname(const upb_fielddef *f, char *buf, size_t len); const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f); const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f); upb_msgdef *upb_fielddef_containingtype_mutable(upb_fielddef *f); @@ -1335,6 +1350,9 @@ inline const char* FieldDef::name() const { return upb_fielddef_name(this); } inline bool FieldDef::is_extension() const { return upb_fielddef_isextension(this); } +inline size_t FieldDef::GetJsonName(char* buf, size_t len) const { + return upb_fielddef_getjsonname(this, buf, len); +} inline bool FieldDef::lazy() const { return upb_fielddef_lazy(this); } diff --git a/upb/json/parser.c b/upb/json/parser.c index 8a50b3a..3731478 100644 --- a/upb/json/parser.c +++ b/upb/json/parser.c @@ -1609,6 +1609,11 @@ static void add_jsonname_table(upb_json_parsermethod *m, const upb_msgdef* md) { upb_msg_field_iter i; upb_strtable *t; + /* It would be nice to stack-allocate this, but protobufs do not limit the + * length of fields to any reasonable limit. */ + char *buf = NULL; + size_t len = 0; + if (upb_inttable_lookupptr(&m->name_tables, md, NULL)) { return; } @@ -1622,17 +1627,20 @@ static void add_jsonname_table(upb_json_parsermethod *m, const upb_msgdef* md) { !upb_msg_field_done(&i); upb_msg_field_next(&i)) { const upb_fielddef *f = upb_msg_iter_field(&i); - /* It would be nice to stack-allocate this, but protobufs do not limit the - * length of fields to any reasonable limit. */ - char *buf = malloc(strlen(upb_fielddef_name(f)) + 1); - upb_fielddef_getjsonname(f, buf); + size_t field_len = upb_fielddef_getjsonname(f, buf, len); + if (field_len > len) { + buf = realloc(buf, field_len); + len = field_len; + upb_fielddef_getjsonname(f, buf, len); + } upb_strtable_insert(t, buf, upb_value_constptr(f)); - free(buf); if (upb_fielddef_issubmsg(f)) { add_jsonname_table(m, upb_fielddef_msgsubdef(f)); } } + + free(buf); } /* Public API *****************************************************************/ diff --git a/upb/json/parser.rl b/upb/json/parser.rl index b85fd21..4ccca6c 100644 --- a/upb/json/parser.rl +++ b/upb/json/parser.rl @@ -1344,6 +1344,11 @@ static void add_jsonname_table(upb_json_parsermethod *m, const upb_msgdef* md) { upb_msg_field_iter i; upb_strtable *t; + /* It would be nice to stack-allocate this, but protobufs do not limit the + * length of fields to any reasonable limit. */ + char *buf = NULL; + size_t len = 0; + if (upb_inttable_lookupptr(&m->name_tables, md, NULL)) { return; } @@ -1357,17 +1362,20 @@ static void add_jsonname_table(upb_json_parsermethod *m, const upb_msgdef* md) { !upb_msg_field_done(&i); upb_msg_field_next(&i)) { const upb_fielddef *f = upb_msg_iter_field(&i); - /* It would be nice to stack-allocate this, but protobufs do not limit the - * length of fields to any reasonable limit. */ - char *buf = malloc(strlen(upb_fielddef_name(f)) + 1); - upb_fielddef_getjsonname(f, buf); + size_t field_len = upb_fielddef_getjsonname(f, buf, len); + if (field_len > len) { + buf = realloc(buf, field_len); + len = field_len; + upb_fielddef_getjsonname(f, buf, len); + } upb_strtable_insert(t, buf, upb_value_constptr(f)); - free(buf); if (upb_fielddef_issubmsg(f)) { add_jsonname_table(m, upb_fielddef_msgsubdef(f)); } } + + free(buf); } /* Public API *****************************************************************/ diff --git a/upb/json/printer.c b/upb/json/printer.c index 5d54abf..c3d9bb4 100644 --- a/upb/json/printer.c +++ b/upb/json/printer.c @@ -47,9 +47,10 @@ void freestrpc(void *ptr) { strpc *newstrpc(upb_handlers *h, const upb_fielddef *f) { /* TODO(haberman): handle malloc failure. */ strpc *ret = malloc(sizeof(*ret)); - ret->ptr = malloc(strlen(upb_fielddef_name(f)) + 1); - upb_fielddef_getjsonname(f, ret->ptr); - ret->len = strlen(ret->ptr); + ret->len = upb_fielddef_getjsonname(f, NULL, 0); + ret->ptr = malloc(ret->len); + upb_fielddef_getjsonname(f, ret->ptr, ret->len); + ret->len--; /* NULL */ upb_handlers_addcleanup(h, ret, freestrpc); return ret; -- cgit v1.2.3