Addressed code review comments.

pull/13171/head
Josh Haberman 9 years ago
parent f9afc3e55b
commit 458da2563f
  1. 28
      upb/def.c
  2. 28
      upb/def.h
  3. 18
      upb/json/parser.c
  4. 18
      upb/json/parser.rl
  5. 7
      upb/json/printer.c

@ -722,33 +722,43 @@ const char *upb_fielddef_name(const upb_fielddef *f) {
return upb_def_fullname(upb_fielddef_upcast(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); const char *name = upb_fielddef_name(f);
size_t i, j; size_t src, dst = 0;
bool ucase_next = false; 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: /* Implement the transformation as described in the spec:
* 1. upper case all letters after an underscore. * 1. upper case all letters after an underscore.
* 2. remove all underscores. * 2. remove all underscores.
*/ */
for (i = 0, j = 0; name[i]; i++) { for (src = 0; name[src]; src++) {
if (name[i] == '_') { if (name[src] == '_') {
ucase_next = true; ucase_next = true;
continue; continue;
} }
if (ucase_next) { if (ucase_next) {
buf[j++] = toupper(name[i]); WRITE(toupper(name[src]));
ucase_next = false; ucase_next = false;
} else { } else {
buf[j++] = name[i]; WRITE(name[src]);
} }
} }
buf[j] = '\0'; WRITE('\0');
return true; return dst;
#undef WRITE
} }
const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f) { const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f) {

@ -307,11 +307,26 @@ class upb::FieldDef {
uint32_t number() const; /* Returns 0 if uninitialized. */ uint32_t number() const; /* Returns 0 if uninitialized. */
bool is_extension() const; bool is_extension() const;
/* Get the JSON name for this field. This will copy the JSON name into the /* Copies the JSON name for this field into the given buffer. Returns the
* given buffer, which must have size of at least "strlen(name()) + 1". * actual size of the JSON name, including the NULL terminator. If the
* The string will be NULL-terminated. Returns false if uninitialized. * 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 <class T>
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, /* For UPB_TYPE_MESSAGE fields only where is_tag_delimited() == false,
* indicates whether this field should have lazy parsing handlers that yield * 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_isextension(const upb_fielddef *f);
bool upb_fielddef_lazy(const upb_fielddef *f); bool upb_fielddef_lazy(const upb_fielddef *f);
bool upb_fielddef_packed(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_msgdef *upb_fielddef_containingtype(const upb_fielddef *f);
const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f); const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f);
upb_msgdef *upb_fielddef_containingtype_mutable(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 { inline bool FieldDef::is_extension() const {
return upb_fielddef_isextension(this); 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 { inline bool FieldDef::lazy() const {
return upb_fielddef_lazy(this); return upb_fielddef_lazy(this);
} }

@ -1609,6 +1609,11 @@ static void add_jsonname_table(upb_json_parsermethod *m, const upb_msgdef* md) {
upb_msg_field_iter i; upb_msg_field_iter i;
upb_strtable *t; 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)) { if (upb_inttable_lookupptr(&m->name_tables, md, NULL)) {
return; 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_done(&i);
upb_msg_field_next(&i)) { upb_msg_field_next(&i)) {
const upb_fielddef *f = upb_msg_iter_field(&i); const upb_fielddef *f = upb_msg_iter_field(&i);
/* It would be nice to stack-allocate this, but protobufs do not limit the size_t field_len = upb_fielddef_getjsonname(f, buf, len);
* length of fields to any reasonable limit. */ if (field_len > len) {
char *buf = malloc(strlen(upb_fielddef_name(f)) + 1); buf = realloc(buf, field_len);
upb_fielddef_getjsonname(f, buf); len = field_len;
upb_fielddef_getjsonname(f, buf, len);
}
upb_strtable_insert(t, buf, upb_value_constptr(f)); upb_strtable_insert(t, buf, upb_value_constptr(f));
free(buf);
if (upb_fielddef_issubmsg(f)) { if (upb_fielddef_issubmsg(f)) {
add_jsonname_table(m, upb_fielddef_msgsubdef(f)); add_jsonname_table(m, upb_fielddef_msgsubdef(f));
} }
} }
free(buf);
} }
/* Public API *****************************************************************/ /* Public API *****************************************************************/

@ -1344,6 +1344,11 @@ static void add_jsonname_table(upb_json_parsermethod *m, const upb_msgdef* md) {
upb_msg_field_iter i; upb_msg_field_iter i;
upb_strtable *t; 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)) { if (upb_inttable_lookupptr(&m->name_tables, md, NULL)) {
return; 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_done(&i);
upb_msg_field_next(&i)) { upb_msg_field_next(&i)) {
const upb_fielddef *f = upb_msg_iter_field(&i); const upb_fielddef *f = upb_msg_iter_field(&i);
/* It would be nice to stack-allocate this, but protobufs do not limit the size_t field_len = upb_fielddef_getjsonname(f, buf, len);
* length of fields to any reasonable limit. */ if (field_len > len) {
char *buf = malloc(strlen(upb_fielddef_name(f)) + 1); buf = realloc(buf, field_len);
upb_fielddef_getjsonname(f, buf); len = field_len;
upb_fielddef_getjsonname(f, buf, len);
}
upb_strtable_insert(t, buf, upb_value_constptr(f)); upb_strtable_insert(t, buf, upb_value_constptr(f));
free(buf);
if (upb_fielddef_issubmsg(f)) { if (upb_fielddef_issubmsg(f)) {
add_jsonname_table(m, upb_fielddef_msgsubdef(f)); add_jsonname_table(m, upb_fielddef_msgsubdef(f));
} }
} }
free(buf);
} }
/* Public API *****************************************************************/ /* Public API *****************************************************************/

@ -47,9 +47,10 @@ void freestrpc(void *ptr) {
strpc *newstrpc(upb_handlers *h, const upb_fielddef *f) { strpc *newstrpc(upb_handlers *h, const upb_fielddef *f) {
/* TODO(haberman): handle malloc failure. */ /* TODO(haberman): handle malloc failure. */
strpc *ret = malloc(sizeof(*ret)); strpc *ret = malloc(sizeof(*ret));
ret->ptr = malloc(strlen(upb_fielddef_name(f)) + 1); ret->len = upb_fielddef_getjsonname(f, NULL, 0);
upb_fielddef_getjsonname(f, ret->ptr); ret->ptr = malloc(ret->len);
ret->len = strlen(ret->ptr); upb_fielddef_getjsonname(f, ret->ptr, ret->len);
ret->len--; /* NULL */
upb_handlers_addcleanup(h, ret, freestrpc); upb_handlers_addcleanup(h, ret, freestrpc);
return ret; return ret;

Loading…
Cancel
Save