Organize allocation logic in generator, add pb_bytes_ptr_t.

Allocation decision is now made before the field data type is decided.
This way the data type decisions can more cleanly account for the allocation
type, i.e. FT_DEFAULT logic etc.

Added pb_bytes_ptr_t for pointer-allocated bytes-fields. There is no point
generating separate structs for these, as they would all be of the same type.

4 files changed, 63 insertions, 66 deletions

diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index 69548ad6..d67fe94f 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -189,6 +189,34 @@ class Field:
         else:
             raise NotImplementedError(desc.label)
         
+        # Check if the field can be implemented with static allocation
+        # i.e. whether the data size is known.
+        if desc.type == FieldD.TYPE_STRING and self.max_size is None:
+            can_be_static = False
+        
+        if desc.type == FieldD.TYPE_BYTES and self.max_size is None:
+            can_be_static = False
+        
+        # Decide how the field data will be allocated
+        if field_options.type == nanopb_pb2.FT_DEFAULT:
+            if can_be_static:
+                field_options.type = nanopb_pb2.FT_STATIC
+            else:
+                field_options.type = nanopb_pb2.FT_CALLBACK
+        
+        if field_options.type == nanopb_pb2.FT_STATIC and not can_be_static:
+            raise Exception("Field %s is defined as static, but max_size or "
+                            "max_count is not given." % self.name)
+        
+        if field_options.type == nanopb_pb2.FT_STATIC:
+            self.allocation = 'STATIC'
+        elif field_options.type == nanopb_pb2.FT_POINTER:
+            self.allocation = 'POINTER'
+        elif field_options.type == nanopb_pb2.FT_CALLBACK:
+            self.allocation = 'CALLBACK'
+        else:
+            raise NotImplementedError(field_options.type)
+        
         # Decide the C data type to use in the struct.
         if datatypes.has_key(desc.type):
             self.ctype, self.pbtype, self.enc_size = datatypes[desc.type]
@@ -200,27 +228,18 @@ class Field:
             self.enc_size = 5 # protoc rejects enum values > 32 bits
         elif desc.type == FieldD.TYPE_STRING:
             self.pbtype = 'STRING'
-            if field_options.type == nanopb_pb2.FT_POINTER:
+            self.ctype = 'char'
+            if self.allocation == 'STATIC':
                 self.ctype = 'char'
-                self.enc_size = None
-            else:
-                if self.max_size is None:
-                    can_be_static = False
-                else:
-                    self.ctype = 'char'
-                    self.array_decl += '[%d]' % self.max_size
-                    self.enc_size = varint_max_size(self.max_size) + self.max_size
+                self.array_decl += '[%d]' % self.max_size
+                self.enc_size = varint_max_size(self.max_size) + self.max_size
         elif desc.type == FieldD.TYPE_BYTES:
             self.pbtype = 'BYTES'
-            if field_options.type == nanopb_pb2.FT_POINTER:
+            if self.allocation == 'STATIC':
                 self.ctype = self.struct_name + self.name + 't'
-                self.enc_size = None
-            else:
-                if self.max_size is None:
-                    can_be_static = False
-                else:
-                    self.ctype = self.struct_name + self.name + 't'
-                    self.enc_size = varint_max_size(self.max_size) + self.max_size
+                self.enc_size = varint_max_size(self.max_size) + self.max_size
+            elif self.allocation == 'POINTER':
+                self.ctype = 'pb_bytes_ptr_t'
         elif desc.type == FieldD.TYPE_MESSAGE:
             self.pbtype = 'MESSAGE'
             self.ctype = self.submsgname = names_from_type_name(desc.type_name)
@@ -228,26 +247,6 @@ class Field:
         else:
             raise NotImplementedError(desc.type)
         
-        if field_options.type == nanopb_pb2.FT_DEFAULT:
-            if can_be_static:
-                field_options.type = nanopb_pb2.FT_STATIC
-            else:
-                field_options.type = nanopb_pb2.FT_CALLBACK
-        
-        if field_options.type == nanopb_pb2.FT_STATIC and not can_be_static:
-            raise Exception("Field %s is defined as static, but max_size or max_count is not given." % self.name)
-        
-        if field_options.type == nanopb_pb2.FT_STATIC:
-            self.allocation = 'STATIC'
-        elif field_options.type == nanopb_pb2.FT_POINTER:
-            self.allocation = 'POINTER'
-        elif field_options.type == nanopb_pb2.FT_CALLBACK:
-            self.allocation = 'CALLBACK'
-            self.ctype = 'pb_callback_t'
-            self.array_decl = ''
-        else:
-            raise NotImplementedError(field_options.type)
-    
     def __cmp__(self, other):
         return cmp(self.tag, other.tag)
     
@@ -257,15 +256,16 @@ class Field:
             if self.rules == 'REPEATED':
                 result += '    size_t ' + self.name + '_count;\n'
             
-            # Use struct definition, so recursive submessages are possible
             if self.pbtype == 'MESSAGE':
+                # Use struct definition, so recursive submessages are possible
                 result += '    struct _%s *%s;' % (self.ctype, self.name)
-
-            # String arrays need to be defined as pointers to pointers
             elif self.rules == 'REPEATED' and self.pbtype == 'STRING':
+                # String arrays need to be defined as pointers to pointers
                 result += '    %s **%s;' % (self.ctype, self.name)
             else:
                 result += '    %s *%s;' % (self.ctype, self.name)
+        elif self.allocation == 'CALLBACK':
+            result += '    pb_callback_t %s;' % self.name
         else:
             if self.rules == 'OPTIONAL' and self.allocation == 'STATIC':
                 result += '    bool has_' + self.name + ';\n'
@@ -276,13 +276,10 @@ class Field:
     
     def types(self):
         '''Return definitions for any special types this field might need.'''
-        if self.pbtype == 'BYTES' and (self.allocation == 'STATIC' or self.allocation == 'POINTER'):
+        if self.pbtype == 'BYTES' and self.allocation == 'STATIC':
             result = 'typedef struct {\n'
             result += '    size_t size;\n'
-            if self.allocation == 'POINTER':
-                result += '    uint8_t *bytes;\n'
-            else:
-                result += '    uint8_t bytes[%d];\n' % self.max_size
+            result += '    uint8_t bytes[%d];\n' % self.max_size
             result += '} %s;\n' % self.ctype
         else:
             result = None
diff --git a/pb.h b/pb.h
index d839be59..76980e4c 100644
--- a/pb.h
+++ b/pb.h
@@ -228,9 +228,17 @@ 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.
@@ -377,10 +385,10 @@ struct _pb_extension_t {
     {tag, PB_ATYPE_POINTER | PB_HTYPE_OPTIONAL | ltype, \
     fd, 0, pb_membersize(st, m[0]), 0, ptr}
 
+/* Repeated fields have a _count field and a pointer to array of pointers */
 #define PB_REPEATED_POINTER(tag, st, m, fd, ltype, ptr) \
     {tag, PB_ATYPE_POINTER | PB_HTYPE_REPEATED | ltype, \
-    fd, \
-    pb_delta(st, m ## _count, m), \
+    fd, pb_delta(st, m ## _count, m), \
     pb_membersize(st, m[0]), 0, ptr}
 
 /* Callbacks are much like required fields except with special datatype. */
diff --git a/pb_encode.c b/pb_encode.c
index cbf7a667..8f01b10b 100644
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -598,14 +598,14 @@ 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)
 {
-    const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)src;
-
     if (PB_ATYPE(field->type) == PB_ATYPE_POINTER)
     {
-        return pb_encode_string(stream, *(const uint8_t**)bytes->bytes, bytes->size);
+        const pb_bytes_ptr_t *bytes = (const pb_bytes_ptr_t*)src;
+        return pb_encode_string(stream, bytes->bytes, bytes->size);
     }
     else
     {
+        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");
 
diff --git a/tests/alltypes_pointer/encode_alltypes_pointer.c b/tests/alltypes_pointer/encode_alltypes_pointer.c
index cb2fe3d5..7e648e39 100644
--- a/tests/alltypes_pointer/encode_alltypes_pointer.c
+++ b/tests/alltypes_pointer/encode_alltypes_pointer.c
@@ -24,9 +24,8 @@ int main(int argc, char **argv)
     double value_double = 1000.0f;
 
     char *value_string = "1000";
-    AllTypes_req_bytes_t value_req_bytes;
-    AllTypes_rep_bytes_t value_rep_bytes;
-    AllTypes_opt_bytes_t value_opt_bytes;
+    
+    pb_bytes_ptr_t value_bytes = {4, (uint8_t*)"1000"};
 
     SubMessage value_submessage = {0};
     MyEnum value_enum = MyEnum_Truth;
@@ -51,11 +50,9 @@ int main(int argc, char **argv)
     alltypes.req_sfixed64      = &value_int64;
     alltypes.req_double        = &value_double;
 
-    value_req_bytes.bytes = (uint8_t*)"1000";
-    value_req_bytes.size  = 4;
-    
     alltypes.req_string        = value_string;
-    alltypes.req_bytes         = &value_req_bytes;
+    
+    alltypes.req_bytes         = &value_bytes;
 
     value_submessage.substuff1 = value_string;
     value_submessage.substuff2 = &value_int32;
@@ -80,11 +77,8 @@ int main(int argc, char **argv)
     alltypes.rep_sfixed64_count = 1; alltypes.rep_sfixed64 = &value_int64;
     alltypes.rep_double_count = 1; alltypes.rep_double = &value_double;
 
-    value_rep_bytes.bytes = (uint8_t*)"1000";
-    value_rep_bytes.size  = 4;
-
     alltypes.rep_string_count = 1; alltypes.rep_string = (char **)&value_string;
-    alltypes.rep_bytes_count = 0; alltypes.rep_bytes = &value_rep_bytes;
+    alltypes.rep_bytes_count = 1; alltypes.rep_bytes = &value_bytes;
 
     alltypes.rep_submsg_count = 1; alltypes.rep_submsg = &value_submessage;
     alltypes.rep_enum_count = 1; alltypes.rep_enum = &value_enum;
@@ -109,11 +103,9 @@ int main(int argc, char **argv)
       alltypes.opt_sfixed64      = &value_int64;
       alltypes.opt_double        = &value_double;
     
-      value_opt_bytes.bytes = (uint8_t*)"1000";
-      value_opt_bytes.size  = 4;
-    
       alltypes.opt_string        = value_string;
-      alltypes.opt_bytes         = &value_opt_bytes;
+      
+      alltypes.opt_bytes         = &value_bytes;
 
       alltypes.opt_submsg        = &value_submessage;
       alltypes.opt_enum          = &value_enum;