Responded to PR comments.

4 files changed, 15 insertions, 12 deletions

diff --git a/tests/conformance_upb.c b/tests/conformance_upb.c
index e1221b2..2ef8c6d 100644
--- a/tests/conformance_upb.c
+++ b/tests/conformance_upb.c
@@ -114,7 +114,7 @@ bool DoTestIo() {
   char *serialized_input;
   char *serialized_output;
   uint32_t input_size;
-  size_t output_size = 0;
+  size_t output_size;
   conformance_ConformanceRequest *request;
   conformance_ConformanceResponse *response;
 
diff --git a/tools/make_c_api.lua b/tools/make_c_api.lua
index cfcf38d..552e861 100644
--- a/tools/make_c_api.lua
+++ b/tools/make_c_api.lua
@@ -155,6 +155,8 @@ local function field_layout_rank(field)
   --  1. padding alignment is (nearly) minimized.
   --  2. fields that might have defaults (1-4) are segregated
   --     from fields that are always zero-initialized (5-7).
+  --
+  -- We skip oneof fields, because they are emitted in a separate pass.
   local rank
   if field:containing_oneof() then
     rank = 100  -- These go last (actually we skip them).
@@ -347,7 +349,7 @@ local function write_c_file(filedef, hfilename, append)
       end
 
       if field:containing_oneof() then
-        -- Do nothing now
+        -- Handled below.
       else
         if has_hasbit(field) then
           hasbit_indexes[field] = hasbit_count
@@ -358,14 +360,11 @@ local function write_c_file(filedef, hfilename, append)
       end
     end
 
-    local oneof_last_fields = {}
     -- Oneof fields.
     for oneof in msg:oneofs() do
       local fullname = to_cident(oneof:containing_type():full_name() .. "." .. oneof:name())
       append('  union {\n')
-      oneof_last_fields[oneof] = ""
       for field in oneof:fields() do
-        oneof_last_fields[oneof] = field:name()
         append('    %s %s;\n', ctype(field), field:name())
       end
       append('  } %s;\n', oneof:name())
@@ -425,7 +424,6 @@ local function write_c_file(filedef, hfilename, append)
         if field:containing_oneof() then
           oneof_index = oneof_indexes[field:containing_oneof()]
         end
-        -- TODO(haberman): oneofs.
         append('  {%s, offsetof(%s, %s), %s, %s, %s, %s, %s},\n',
                field:number(),
                msgname,
diff --git a/upb/decode.c b/upb/decode.c
index 3a44021..b09da42 100644
--- a/upb/decode.c
+++ b/upb/decode.c
@@ -54,18 +54,18 @@ static bool upb_decode_message(upb_decstate *d, const char *limit,
 
 static bool upb_decode_varint(const char **ptr, const char *limit,
                               uint64_t *val) {
-  uint8_t byte = 0x80;
+  uint8_t byte;
   int bitpos = 0;
   const char *p = *ptr;
   *val = 0;
 
-  while (byte & 0x80) {
+  do {
     CHK(bitpos < 70 && p < limit);
     byte = *p;
     *val |= (uint64_t)(byte & 0x7F) << bitpos;
     p++;
     bitpos += 7;
-  }
+  } while (byte & 0x80);
 
   *ptr = p;
   return true;
@@ -385,7 +385,7 @@ static bool upb_decode_fixedpacked(upb_array *arr, upb_stringview data,
   int elements = data.size / elem_size;
   void *field_mem;
 
-  CHK((data.size % elem_size) == 0);
+  CHK(elements * elem_size == data.size);
   field_mem = upb_array_add(arr, elements);
   CHK(field_mem);
   memcpy(field_mem, data.data, data.size);
@@ -451,6 +451,7 @@ static bool upb_decode_toarray(upb_decstate *d, upb_decframe *frame,
       return upb_append_unknown(d, frame, field_start);
   }
 #undef VARINT_CASE
+  UPB_UNREACHABLE();
 }
 
 static bool upb_decode_delimitedfield(upb_decstate *d, upb_decframe *frame,
diff --git a/upb/encode.c b/upb/encode.c
index 4aafb9c..e7585eb 100644
--- a/upb/encode.c
+++ b/upb/encode.c
@@ -32,7 +32,7 @@ static const uint8_t upb_desctype_to_fieldtype[] = {
 
 static size_t upb_encode_varint(uint64_t val, char *buf) {
   size_t i;
-  if (val == 0) { buf[0] = 0; return 1; }
+  if (val < 128) { buf[0] = val; return 1; }
   i = 0;
   while (val) {
     uint8_t byte = val & 0x7fU;
@@ -373,7 +373,11 @@ char *upb_encode(const void *msg, const upb_msglayout_msginit_v1 *m,
   e.limit = NULL;
   e.ptr = NULL;
 
-  CHK(upb_encode_message(&e, msg, m, size));
+  if (!upb_encode_message(&e, msg, m, size)) {
+    *size = 0;
+    return NULL;
+  }
+
   *size = e.limit - e.ptr;
 
   if (*size == 0) {