Bugfixes for oneof support.

Fixes crashes / memory leaks when using pointer type fields.
Also fixes initialization of which_oneof fields.

3 files changed, 24 insertions, 15 deletions

diff --git a/pb.h b/pb.h
index af58dca3..fc846c94 100644
--- a/pb.h
+++ b/pb.h
@@ -514,7 +514,7 @@ struct pb_extension_s {
 #define PB_ONEOF_POINTER(u, tag, st, m, fd, ltype, ptr) \
     {tag, PB_ATYPE_POINTER | PB_HTYPE_ONEOF | ltype, \
     fd, pb_delta(st, which_ ## u, u.m), \
-    pb_membersize(st, u.m), 0, ptr}
+    pb_membersize(st, u.m[0]), 0, ptr}
 
 #define PB_ONEOF_FIELD(union_name, tag, type, rules, allocation, placement, message, field, prevfield, ptr) \
         PB_ ## rules ## _ ## allocation(union_name, tag, message, field, \
diff --git a/pb_common.c b/pb_common.c
index 98964850..385c0193 100644
--- a/pb_common.c
+++ b/pb_common.c
@@ -41,8 +41,15 @@ bool pb_field_iter_next(pb_field_iter_t *iter)
         /* Increment the pointers based on previous field size */
         size_t prev_size = prev_field->data_size;
     
-        if (PB_ATYPE(prev_field->type) == PB_ATYPE_STATIC &&
-            PB_HTYPE(prev_field->type) == PB_HTYPE_REPEATED)
+        if (PB_HTYPE(prev_field->type) == PB_HTYPE_ONEOF &&
+            PB_HTYPE(iter->pos->type) == PB_HTYPE_ONEOF)
+        {
+            /* Don't advance pointers inside unions */
+            prev_size = 0;
+            iter->pData = (char*)iter->pData - prev_field->data_offset;
+        }
+        else if (PB_ATYPE(prev_field->type) == PB_ATYPE_STATIC &&
+                 PB_HTYPE(prev_field->type) == PB_HTYPE_REPEATED)
         {
             /* In static arrays, the data_size tells the size of a single entry and
              * array_size is the number of entries */
@@ -54,14 +61,7 @@ bool pb_field_iter_next(pb_field_iter_t *iter)
              * The data_size only applies to the dynamically allocated area. */
             prev_size = sizeof(void*);
         }
-        else if (PB_HTYPE(prev_field->type) == PB_HTYPE_ONEOF &&
-                 PB_HTYPE(iter->pos->type) == PB_HTYPE_ONEOF)
-        {
-            /* Don't advance pointers inside unions */
-            prev_size = 0;
-            iter->pData = (char*)iter->pData - prev_field->data_offset;
-        }
-        
+
         if (PB_HTYPE(prev_field->type) == PB_HTYPE_REQUIRED)
         {
             /* Count the required fields, in order to check their presence in the
diff --git a/pb_decode.c b/pb_decode.c
index 542fdc4c..0677594f 100644
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -747,13 +747,15 @@ static void pb_field_set_to_default(pb_field_iter_t *iter)
              * itself also. */
             *(bool*)iter->pSize = false;
         }
-        else if (PB_HTYPE(type) == PB_HTYPE_REPEATED)
+        else if (PB_HTYPE(type) == PB_HTYPE_REPEATED ||
+                 PB_HTYPE(type) == PB_HTYPE_ONEOF)
         {
-            /* Set array count to 0, no need to initialize contents. */
+            /* REPEATED: Set array count to 0, no need to initialize contents.
+               ONEOF: Set which_field to 0. */
             *(pb_size_t*)iter->pSize = 0;
             init_data = false;
         }
-        
+
         if (init_data)
         {
             if (PB_LTYPE(iter->pos->type) == PB_LTYPE_SUBMESSAGE)
@@ -779,7 +781,8 @@ static void pb_field_set_to_default(pb_field_iter_t *iter)
         *(void**)iter->pData = NULL;
         
         /* Initialize array count to 0. */
-        if (PB_HTYPE(type) == PB_HTYPE_REPEATED)
+        if (PB_HTYPE(type) == PB_HTYPE_REPEATED ||
+            PB_HTYPE(type) == PB_HTYPE_ONEOF)
         {
             *(pb_size_t*)iter->pSize = 0;
         }
@@ -938,6 +941,12 @@ static void pb_release_single_field(const pb_field_iter_t *iter)
     pb_type_t type;
     type = iter->pos->type;
 
+    if (PB_HTYPE(type) == PB_HTYPE_ONEOF)
+    {
+        if (*(pb_size_t*)iter->pSize != iter->pos->tag)
+            return; /* This is not the current field in the union */
+    }
+
     /* Release anything contained inside an extension or submsg.
      * This has to be done even if the submsg itself is statically
      * allocated. */