From 81c2aa753e2bf5681393c5e5120232f9b10dfb85 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Thu, 11 Jun 2020 12:42:17 -0700 Subject: [PATCH 1/2] Fixes for the PHP C Extension. --- upb/def.c | 24 ++++++++++++++++++++---- upb/json_encode.c | 19 ++++++++++++++++--- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/upb/def.c b/upb/def.c index 10f6b94bf1..b0d6f876c8 100644 --- a/upb/def.c +++ b/upb/def.c @@ -922,6 +922,22 @@ static uint32_t upb_msglayout_place(upb_msglayout *l, size_t size) { return ret; } +static int field_number_cmp(const void *p1, const void *p2) { + const upb_msglayout_field *f1 = p1; + const upb_msglayout_field *f2 = p2; + return f1->number - f2->number; +} + +static void assign_layout_indices(const upb_msgdef *m, upb_msglayout_field *fields) { + int i; + int n = upb_msgdef_numfields(m); + for (i = 0; i < n; i++) { + upb_fielddef *f = (upb_fielddef*)upb_msgdef_itof(m, fields[i].number); + UPB_ASSERT(f); + f->layout_index = i; + } +} + /* This function is the dynamic equivalent of message_layout.{cc,h} in upbc. * It computes a dynamic layout for all of the fields in |m|. */ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) { @@ -1003,10 +1019,6 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) { field->label = _UPB_LABEL_PACKED; } - /* TODO: we probably should sort the fields by field number to match the - * output of upbc, and to improve search speed for the table parser. */ - f->layout_index = f->index_; - if (upb_fielddef_issubmsg(f)) { const upb_msgdef *subm = upb_fielddef_msgsubdef(f); field->submsg_index = submsg_count++; @@ -1079,6 +1091,10 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) { * alignment. TODO: track overall alignment for real? */ l->size = UPB_ALIGN_UP(l->size, 8); + /* Sort fields by number. */ + qsort(fields, upb_msgdef_numfields(m), sizeof(*fields), field_number_cmp); + assign_layout_indices(m, fields); + return true; } diff --git a/upb/json_encode.c b/upb/json_encode.c index 21f661f8f6..58b7656455 100644 --- a/upb/json_encode.c +++ b/upb/json_encode.c @@ -38,6 +38,14 @@ UPB_NORETURN static void jsonenc_err(jsonenc *e, const char *msg) { longjmp(e->err, 1); } +UPB_NORETURN static void jsonenc_errf(jsonenc *e, const char *fmt, ...) { + va_list argp; + va_start(argp, fmt); + upb_status_vseterrf(e->status, fmt, argp); + va_end(argp); + longjmp(e->err, 1); +} + static upb_arena *jsonenc_arena(jsonenc *e) { /* Create lazily, since it's only needed for Any */ if (!e->arena) { @@ -279,7 +287,11 @@ static const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) { const char *ptr = end; const upb_msgdef *ret; - if (!e->ext_pool || type_url.size == 0) goto badurl; + if (!e->ext_pool) { + jsonenc_err(e, "Tried to encode Any, but no symtab was provided"); + } + + if (type_url.size == 0) goto badurl; while (true) { if (--ptr == type_url.data) { @@ -295,13 +307,14 @@ static const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) { ret = upb_symtab_lookupmsg2(e->ext_pool, ptr, end - ptr); if (!ret) { - jsonenc_err(e, "Couldn't find Any type"); + jsonenc_errf(e, "Couldn't find Any type: %.*s (full URL: " UPB_STRVIEW_FORMAT ")", (int)(end - ptr), ptr, UPB_STRVIEW_ARGS(type_url)); } return ret; badurl: - jsonenc_err(e, "Bad type URL"); + jsonenc_errf( + e, "Bad type URL: " UPB_STRVIEW_FORMAT, UPB_STRVIEW_ARGS(type_url)); } static void jsonenc_any(jsonenc *e, const upb_msg *msg, const upb_msgdef *m) { From efe11c6c504a040f40f2b442ccadf58e9b76b7a7 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Thu, 11 Jun 2020 13:01:51 -0700 Subject: [PATCH 2/2] Removed excess logging statement. --- upb/json_encode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upb/json_encode.c b/upb/json_encode.c index 58b7656455..6b6d99b30f 100644 --- a/upb/json_encode.c +++ b/upb/json_encode.c @@ -307,7 +307,7 @@ static const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) { ret = upb_symtab_lookupmsg2(e->ext_pool, ptr, end - ptr); if (!ret) { - jsonenc_errf(e, "Couldn't find Any type: %.*s (full URL: " UPB_STRVIEW_FORMAT ")", (int)(end - ptr), ptr, UPB_STRVIEW_ARGS(type_url)); + jsonenc_errf(e, "Couldn't find Any type: %.*s", (int)(end - ptr), ptr); } return ret;