Get rid of pb_bytes_ptr_t, just allocate pb_bytes_array_t dynamically.

This makes the internal logic much simpler, and also keeps the datatypes
more similar between STATIC/POINTER cases. It will still be a bit cumbersome
to use because of variable length array member. Macros PB_BYTES_ARRAY_T(n) and
PB_BYTES_ARRAY_T_ALLOCSIZE(n) have been added to make life a bit easier.

This has the drawback that it is no longer as easy to use externally allocated
byte array as input for bytes field in pointer mode. However, this is still
easy to do using callbacks, so it shouldn't be a large issue.

7 files changed, 59 insertions, 77 deletions

diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index 0926db29..c32b26ad 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -246,7 +246,7 @@ class Field:
                 self.ctype = self.struct_name + self.name + 't'
                 self.enc_size = varint_max_size(self.max_size) + self.max_size
             elif self.allocation == 'POINTER':
-                self.ctype = 'pb_bytes_ptr_t'
+                self.ctype = 'pb_bytes_array_t'
         elif desc.type == FieldD.TYPE_MESSAGE:
             self.pbtype = 'MESSAGE'
             self.ctype = self.submsgname = names_from_type_name(desc.type_name)
@@ -266,8 +266,8 @@ class Field:
             if self.pbtype == 'MESSAGE':
                 # Use struct definition, so recursive submessages are possible
                 result += '    struct _%s *%s;' % (self.ctype, self.name)
-            elif self.rules == 'REPEATED' and self.pbtype == 'STRING':
-                # String arrays need to be defined as pointers to pointers
+            elif self.rules == 'REPEATED' and self.pbtype in ['STRING', 'BYTES']:
+                # String/bytes arrays need to be defined as pointers to pointers
                 result += '    %s **%s;' % (self.ctype, self.name)
             else:
                 result += '    %s *%s;' % (self.ctype, self.name)
diff --git a/pb.h b/pb.h
index eb4f94e7..d6cb1d40 100644
--- a/pb.h
+++ b/pb.h
@@ -238,21 +238,15 @@ STATIC_ASSERT(sizeof(uint64_t) == 8, UINT64_T_WRONG_SIZE)
  * It has the number of bytes in the beginning, and after that an array.
  * Note that actual structs used will have a different length of bytes array.
  */
+#define PB_BYTES_ARRAY_T(n) struct { size_t size; uint8_t bytes[n]; }
+#define PB_BYTES_ARRAY_T_ALLOCSIZE(n) ((size_t)n + offsetof(pb_bytes_array_t, bytes))
+
 struct _pb_bytes_array_t {
     size_t size;
     uint8_t bytes[1];
 };
 typedef struct _pb_bytes_array_t pb_bytes_array_t;
 
-/* Same, except for pointer-type fields. There is no need to variable struct
- * length in this case.
- */
-struct _pb_bytes_ptr_t {
-    size_t size;
-    uint8_t *bytes;
-};
-typedef struct _pb_bytes_ptr_t pb_bytes_ptr_t;
-
 /* This structure is used for giving the callback function.
  * It is stored in the message structure and filled in by the method that
  * calls pb_decode.
diff --git a/pb_decode.c b/pb_decode.c
index 68497406..81febb9b 100644
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -491,13 +491,10 @@ static bool checkreturn allocate_field(pb_istream_t *stream, void *pData, size_t
 /* Clear a newly allocated item in case it contains a pointer, or is a submessage. */
 static void initialize_pointer_field(void *pItem, pb_field_iterator_t *iter)
 {
-    if (PB_LTYPE(iter->pos->type) == PB_LTYPE_STRING)
+    if (PB_LTYPE(iter->pos->type) == PB_LTYPE_STRING ||
+        PB_LTYPE(iter->pos->type) == PB_LTYPE_BYTES)
     {
-        *(char**)pItem = NULL;
-    }
-    else if (PB_LTYPE(iter->pos->type) == PB_LTYPE_BYTES)
-    {
-        memset(pItem, 0, iter->pos->data_size); 
+        *(void**)pItem = NULL;
     }
     else if (PB_LTYPE(iter->pos->type) == PB_LTYPE_SUBMESSAGE)
     {
@@ -523,7 +520,8 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_
     {
         case PB_HTYPE_REQUIRED:
         case PB_HTYPE_OPTIONAL:
-            if (PB_LTYPE(type) == PB_LTYPE_STRING)
+            if (PB_LTYPE(type) == PB_LTYPE_STRING ||
+                PB_LTYPE(type) == PB_LTYPE_BYTES)
             {
                 return func(stream, iter->pos, iter->pData);
             }
@@ -930,10 +928,11 @@ void pb_release(const pb_field_t fields[], void *dest_struct)
         
         if (PB_ATYPE(type) == PB_ATYPE_POINTER)
         {
-            if (PB_LTYPE(type) == PB_LTYPE_STRING &&
-                PB_HTYPE(type) == PB_HTYPE_REPEATED)
+            if (PB_HTYPE(type) == PB_HTYPE_REPEATED &&
+                (PB_LTYPE(type) == PB_LTYPE_STRING ||
+                 PB_LTYPE(type) == PB_LTYPE_BYTES))
             {
-                /* Release entries in repeated string array */
+                /* Release entries in repeated string or bytes array */
                 void **pItem = *(void***)iter.pData;
                 size_t count = *(size_t*)iter.pSize;
                 while (count--)
@@ -942,24 +941,6 @@ void pb_release(const pb_field_t fields[], void *dest_struct)
                     *pItem++ = NULL;
                 }
             }
-            else if (PB_LTYPE(type) == PB_LTYPE_BYTES)
-            {
-                /* Release entries in repeated bytes array */
-                pb_bytes_ptr_t *pItem = *(pb_bytes_ptr_t**)iter.pData;
-                size_t count = (pItem ? 1 : 0);
-                
-                if (PB_HTYPE(type) == PB_HTYPE_REPEATED)
-                {
-                    count = *(size_t*)iter.pSize;   
-                }
-                
-                while (count--)
-                {
-                    free(pItem->bytes);
-                    pItem->bytes = NULL;
-                    pItem++;
-                }
-            }
             else if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE)
             {
                 /* Release fields in submessages */
@@ -1109,35 +1090,30 @@ bool checkreturn pb_dec_fixed64(pb_istream_t *stream, const pb_field_t *field, v
 bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest)
 {
     uint32_t size;
-    size_t alloc_size;
+    pb_bytes_array_t *bdest;
     
     if (!pb_decode_varint32(stream, &size))
         return false;
     
-    /* Space for the size_t header */
-    alloc_size = size + offsetof(pb_bytes_array_t, bytes);
-    
     if (PB_ATYPE(field->type) == PB_ATYPE_POINTER)
     {
 #ifndef PB_ENABLE_MALLOC
         PB_RETURN_ERROR(stream, "no malloc support");
 #else
-        pb_bytes_ptr_t *bdest = (pb_bytes_ptr_t*)dest;
-        if (!allocate_field(stream, &bdest->bytes, alloc_size, 1))
+        if (!allocate_field(stream, dest, PB_BYTES_ARRAY_T_ALLOCSIZE(size), 1))
             return false;
-        
-        bdest->size = size;
-        return pb_read(stream, bdest->bytes, size);
+        bdest = *(pb_bytes_array_t**)dest;
 #endif
     }
     else
     {
-        pb_bytes_array_t* bdest = (pb_bytes_array_t*)dest;
-        if (alloc_size > field->data_size)
+        if (PB_BYTES_ARRAY_T_ALLOCSIZE(size) > field->data_size)
             PB_RETURN_ERROR(stream, "bytes overflow");
-        bdest->size = size;
-        return pb_read(stream, bdest->bytes, size);
+        bdest = (pb_bytes_array_t*)dest;
     }
+    
+    bdest->size = size;
+    return pb_read(stream, bdest->bytes, size);
 }
 
 bool checkreturn pb_dec_string(pb_istream_t *stream, const pb_field_t *field, void *dest)
diff --git a/pb_encode.c b/pb_encode.c
index c2d0e2c4..59e6f2a7 100644
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -174,11 +174,12 @@ static bool checkreturn encode_array(pb_ostream_t *stream, const pb_field_t *fie
                 return false;
 
             /* Normally the data is stored directly in the array entries, but
-             * for pointer-type string fields, the array entries are actually
-             * string pointers. So we have to dereference once more to get to
-             * the character data. */
+             * for pointer-type string and bytes fields, the array entries are
+             * actually pointers themselves also. So we have to dereference once
+             * more to get to the actual data. */
             if (PB_ATYPE(field->type) == PB_ATYPE_POINTER &&
-                PB_LTYPE(field->type) == PB_LTYPE_STRING)
+                (PB_LTYPE(field->type) == PB_LTYPE_STRING ||
+                 PB_LTYPE(field->type) == PB_LTYPE_BYTES))
             {
                 if (!func(stream, field, *(const void* const*)p))
                     return false;      
@@ -603,19 +604,21 @@ bool checkreturn pb_enc_fixed32(pb_ostream_t *stream, const pb_field_t *field, c
 
 bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src)
 {
-    if (PB_ATYPE(field->type) == PB_ATYPE_POINTER)
+    const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)src;
+    
+    if (src == NULL)
     {
-        const pb_bytes_ptr_t *bytes = (const pb_bytes_ptr_t*)src;
-        return pb_encode_string(stream, bytes->bytes, bytes->size);
+        /* Threat null pointer as an empty bytes field */
+        return pb_encode_string(stream, NULL, 0);
     }
-    else
+    
+    if (PB_ATYPE(field->type) == PB_ATYPE_STATIC &&
+        PB_BYTES_ARRAY_T_ALLOCSIZE(bytes->size) > field->data_size)
     {
-        const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)src;
-        if (bytes->size + offsetof(pb_bytes_array_t, bytes) > field->data_size)
-            PB_RETURN_ERROR(stream, "bytes size exceeded");
-
-        return pb_encode_string(stream, bytes->bytes, bytes->size);
+        PB_RETURN_ERROR(stream, "bytes size exceeded");
     }
+    
+    return pb_encode_string(stream, bytes->bytes, bytes->size);
 }
 
 bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *field, const void *src)
@@ -628,10 +631,17 @@ bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *field, co
     if (PB_ATYPE(field->type) == PB_ATYPE_POINTER)
         max_size = (size_t)-1;
 
-    while (size < max_size && *p != '\0')
+    if (src == NULL)
     {
-        size++;
-        p++;
+        size = 0; /* Threat null pointer as an empty string */
+    }
+    else
+    {
+        while (size < max_size && *p != '\0')
+        {
+            size++;
+            p++;
+        }
     }
 
     return pb_encode_string(stream, (const uint8_t*)src, size);
diff --git a/tests/alltypes_pointer/SConscript b/tests/alltypes_pointer/SConscript
index 45985ff5..97e4267a 100644
--- a/tests/alltypes_pointer/SConscript
+++ b/tests/alltypes_pointer/SConscript
@@ -30,9 +30,11 @@ refdec = "$BUILD/alltypes/decode_alltypes$PROGSUFFIX"
 
 # Encode and compare results
 env.RunTest(enc)
+env.Compare(["encode_alltypes_pointer.output", "$BUILD/alltypes/encode_alltypes.output"])
+
+# Decode
 env.RunTest("decode_alltypes.output", [dec, "encode_alltypes_pointer.output"])
 env.RunTest("decode_alltypes_ref.output", [refdec, "encode_alltypes_pointer.output"])
-env.Compare(["encode_alltypes_pointer.output", "$BUILD/alltypes/encode_alltypes.output"])
 
 # Do the same thing with the optional fields present
 env.RunTest("optionals.output", enc, ARGS = ['1'])
diff --git a/tests/alltypes_pointer/decode_alltypes_pointer.c b/tests/alltypes_pointer/decode_alltypes_pointer.c
index 29495ac4..47d72685 100644
--- a/tests/alltypes_pointer/decode_alltypes_pointer.c
+++ b/tests/alltypes_pointer/decode_alltypes_pointer.c
@@ -48,8 +48,7 @@ bool check_alltypes(pb_istream_t *stream, int mode)
     
     TEST(alltypes.req_string    && strcmp(alltypes.req_string, "1014") == 0);
     TEST(alltypes.req_bytes     && alltypes.req_bytes->size == 4);
-    TEST(alltypes.req_bytes     && alltypes.req_bytes->bytes
-                                && memcmp(alltypes.req_bytes->bytes, "1015", 4) == 0);
+    TEST(alltypes.req_bytes     && memcmp(&alltypes.req_bytes->bytes, "1015", 4) == 0);
     TEST(alltypes.req_submsg    && alltypes.req_submsg->substuff1
                                 && strcmp(alltypes.req_submsg->substuff1, "1016") == 0);
     TEST(alltypes.req_submsg    && alltypes.req_submsg->substuff2
diff --git a/tests/alltypes_pointer/encode_alltypes_pointer.c b/tests/alltypes_pointer/encode_alltypes_pointer.c
index 6484ced8..c128569b 100644
--- a/tests/alltypes_pointer/encode_alltypes_pointer.c
+++ b/tests/alltypes_pointer/encode_alltypes_pointer.c
@@ -27,7 +27,7 @@ int main(int argc, char **argv)
     int64_t     req_sfixed64      = -1012;
     double      req_double        = 1013.0;
     char*       req_string        = "1014";
-    pb_bytes_ptr_t req_bytes      = {4, (uint8_t*)"1015"};
+    PB_BYTES_ARRAY_T(4) req_bytes = {4, {'1', '0', '1', '5'}};
     static int32_t req_substuff   = 1016;
     SubMessage  req_submsg        = {"1016", &req_substuff};
     MyEnum      req_enum          = MyEnum_Truth;
@@ -50,7 +50,8 @@ int main(int argc, char **argv)
     int64_t     rep_sfixed64[5]   = {0, 0, 0, 0, -2012};
     double      rep_double[5]     = {0, 0, 0, 0, 2013.0f};
     char*       rep_string[5]     = {"", "", "", "", "2014"};
-    pb_bytes_ptr_t rep_bytes[5]   = {{0,0}, {0,0}, {0,0}, {0,0}, {4, (uint8_t*)"2015"}};
+    static PB_BYTES_ARRAY_T(4) rep_bytes_4 = {4, {'2', '0', '1', '5'}};
+    pb_bytes_array_t *rep_bytes[5]= {NULL, NULL, NULL, NULL, (pb_bytes_array_t*)&rep_bytes_4};
     static int32_t rep_sub2zero   = 0;
     static int32_t rep_substuff2  = 2016;
     static uint32_t rep_substuff3 = 2016;
@@ -77,7 +78,7 @@ int main(int argc, char **argv)
     int64_t     opt_sfixed64      = 3052;
     double      opt_double        = 3053.0;
     char*       opt_string        = "3054";
-    pb_bytes_ptr_t opt_bytes      = {4, (uint8_t*)"3055"};
+    PB_BYTES_ARRAY_T(4) opt_bytes = {4, {'3', '0', '5', '5'}};
     static int32_t opt_substuff   = 3056;
     SubMessage  opt_submsg        = {"3056", &opt_substuff};
     MyEnum      opt_enum          = MyEnum_Truth;
@@ -117,7 +118,7 @@ int main(int argc, char **argv)
     alltypes.req_sfixed64      = &req_sfixed64;
     alltypes.req_double        = &req_double;
     alltypes.req_string        = req_string;
-    alltypes.req_bytes         = &req_bytes;
+    alltypes.req_bytes         = (pb_bytes_array_t*)&req_bytes;
     alltypes.req_submsg        = &req_submsg;
     alltypes.req_enum          = &req_enum;
     alltypes.req_emptymsg      = &req_emptymsg;
@@ -159,7 +160,7 @@ int main(int argc, char **argv)
         alltypes.opt_sfixed64      = &opt_sfixed64;
         alltypes.opt_double        = &opt_double;
         alltypes.opt_string        = opt_string;
-        alltypes.opt_bytes         = &opt_bytes;
+        alltypes.opt_bytes         = (pb_bytes_array_t*)&opt_bytes;
         alltypes.opt_submsg        = &opt_submsg;
         alltypes.opt_enum          = &opt_enum;
         alltypes.opt_emptymsg      = &opt_emptymsg;