Separate PB_HTYPE to PB_ATYPE and PB_HTYPE.

Also clean up the logic so that it is easier to implement more
allocation types in the future.

Update issue 53
Status: FixedInGit

3 files changed, 176 insertions, 122 deletions

diff --git a/pb.h b/pb.h
index cd3f465e..dd06c69b 100644
--- a/pb.h
+++ b/pb.h
@@ -84,29 +84,24 @@ typedef uint8_t pb_type_t;
 #define PB_LTYPES_COUNT 7
 #define PB_LTYPE_MASK 0x0F
 
-/******************
- * Modifier flags *
- ******************/
+/**************************
+ * Field repetition rules *
+ **************************/
 
-/* Just the basic, write data at data_offset */
 #define PB_HTYPE_REQUIRED 0x00
-
-/* Write true at size_offset */
 #define PB_HTYPE_OPTIONAL 0x10
-
-/* Read to pre-allocated array
- * Maximum number of entries is array_size,
- * actual number is stored at size_offset */
 #define PB_HTYPE_REPEATED 0x20
+#define PB_HTYPE_MASK     0x30
 
-/* Works for all required/optional/repeated fields.
- * data_offset points to pb_callback_t structure.
- * LTYPE should be valid or 0 (it is ignored, but
- * sometimes used to speculatively index an array). */
-#define PB_HTYPE_CALLBACK 0x30
-
-#define PB_HTYPE_MASK 0xF0
+/********************
+ * Allocation types *
+ ********************/
+ 
+#define PB_ATYPE_STATIC   0x00
+#define PB_ATYPE_CALLBACK 0x40
+#define PB_ATYPE_MASK     0xC0
 
+#define PB_ATYPE(x) ((x) & PB_ATYPE_MASK)
 #define PB_HTYPE(x) ((x) & PB_HTYPE_MASK)
 #define PB_LTYPE(x) ((x) & PB_LTYPE_MASK)
 
@@ -214,19 +209,19 @@ typedef enum {
  * submessages and default values.
  */
 #define PB_REQUIRED_STATIC(tag, st, m, pm, ltype, ptr) \
-    {tag, PB_HTYPE_REQUIRED | ltype, \
+    {tag, PB_ATYPE_STATIC | PB_HTYPE_REQUIRED | ltype, \
     pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr}
 
 /* Optional fields add the delta to the has_ variable. */
 #define PB_OPTIONAL_STATIC(tag, st, m, pm, ltype, ptr) \
-    {tag, PB_HTYPE_OPTIONAL | ltype, \
+    {tag, PB_ATYPE_STATIC | PB_HTYPE_OPTIONAL | ltype, \
     pb_delta_end(st, m, pm), \
     pb_delta(st, has_ ## m, m), \
     pb_membersize(st, m), 0, ptr}
 
 /* Repeated fields have a _count field and also the maximum number of entries. */
 #define PB_REPEATED_STATIC(tag, st, m, pm, ltype, ptr) \
-    {tag, PB_HTYPE_REPEATED | ltype, \
+    {tag, PB_ATYPE_STATIC | PB_HTYPE_REPEATED | ltype, \
     pb_delta_end(st, m, pm), \
     pb_delta(st, m ## _count, m), \
     pb_membersize(st, m[0]), \
@@ -234,15 +229,15 @@ typedef enum {
 
 /* Callbacks are much like required fields except with special datatype. */
 #define PB_REQUIRED_CALLBACK(tag, st, m, pm, ltype, ptr) \
-    {tag, PB_HTYPE_CALLBACK | ltype, \
+    {tag, PB_ATYPE_CALLBACK | PB_HTYPE_REQUIRED | ltype, \
     pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr}
 
 #define PB_OPTIONAL_CALLBACK(tag, st, m, pm, ltype, ptr) \
-    {tag, PB_HTYPE_CALLBACK | ltype, \
+    {tag, PB_ATYPE_CALLBACK | PB_HTYPE_OPTIONAL | ltype, \
     pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr}
     
 #define PB_REPEATED_CALLBACK(tag, st, m, pm, ltype, ptr) \
-    {tag, PB_HTYPE_CALLBACK | ltype, \
+    {tag, PB_ATYPE_CALLBACK | PB_HTYPE_REPEATED | ltype, \
     pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr}
 
 /* The mapping from protobuf types to LTYPEs is done using these macros. */
diff --git a/pb_decode.c b/pb_decode.c
index 351c8ec7..bc6df7b9 100644
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -303,8 +303,11 @@ static bool pb_field_next(pb_field_iterator_t *iter)
     bool notwrapped = true;
     size_t prev_size = iter->current->data_size;
     
-    if (PB_HTYPE(iter->current->type) == PB_HTYPE_REPEATED)
+    if (PB_ATYPE(iter->current->type) == PB_ATYPE_STATIC &&
+        PB_HTYPE(iter->current->type) == PB_HTYPE_REPEATED)
+    {
         prev_size *= iter->current->array_size;
+    }
     
     if (PB_HTYPE(iter->current->type) == PB_HTYPE_REQUIRED)
         iter->required_field_index++;
@@ -343,11 +346,15 @@ static bool checkreturn pb_field_find(pb_field_iterator_t *iter, uint32_t tag)
  * Decode a single field *
  *************************/
 
-static bool checkreturn decode_field(pb_istream_t *stream, pb_wire_type_t wire_type, pb_field_iterator_t *iter)
+static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t wire_type, pb_field_iterator_t *iter)
 {
-    pb_decoder_t func = PB_DECODERS[PB_LTYPE(iter->current->type)];
+    pb_type_t type;
+    pb_decoder_t func;
     
-    switch (PB_HTYPE(iter->current->type))
+    type = iter->current->type;
+    func = PB_DECODERS[PB_LTYPE(type)];
+
+    switch (PB_HTYPE(type))
     {
         case PB_HTYPE_REQUIRED:
             return func(stream, iter->current, iter->pData);
@@ -358,7 +365,7 @@ static bool checkreturn decode_field(pb_istream_t *stream, pb_wire_type_t wire_t
     
         case PB_HTYPE_REPEATED:
             if (wire_type == PB_WT_STRING
-                && PB_LTYPE(iter->current->type) <= PB_LTYPE_LAST_PACKABLE)
+                && PB_LTYPE(type) <= PB_LTYPE_LAST_PACKABLE)
             {
                 /* Packed array */
                 bool status = true;
@@ -395,48 +402,63 @@ static bool checkreturn decode_field(pb_istream_t *stream, pb_wire_type_t wire_t
                 (*size)++;
                 return func(stream, iter->current, pItem);
             }
+
+        default:
+            PB_RETURN_ERROR(stream, "invalid field type");
+    }
+}
+
+static bool checkreturn decode_callback_field(pb_istream_t *stream, pb_wire_type_t wire_type, pb_field_iterator_t *iter)
+{
+    pb_callback_t *pCallback = (pb_callback_t*)iter->pData;
+    
+    if (pCallback->funcs.decode == NULL)
+        return pb_skip_field(stream, wire_type);
+    
+    if (wire_type == PB_WT_STRING)
+    {
+        pb_istream_t substream;
         
-        case PB_HTYPE_CALLBACK:
+        if (!pb_make_string_substream(stream, &substream))
+            return false;
+        
+        while (substream.bytes_left)
         {
-            pb_callback_t *pCallback = (pb_callback_t*)iter->pData;
-            
-            if (pCallback->funcs.decode == NULL)
-                return pb_skip_field(stream, wire_type);
-            
-            if (wire_type == PB_WT_STRING)
-            {
-                pb_istream_t substream;
-                
-                if (!pb_make_string_substream(stream, &substream))
-                    return false;
-                
-                while (substream.bytes_left)
-                {
-                    if (!pCallback->funcs.decode(&substream, iter->current, pCallback->arg))
-                        PB_RETURN_ERROR(stream, "callback failed");
-                }
-                
-                pb_close_string_substream(stream, &substream);
-                return true;
-            }
-            else
-            {
-                /* Copy the single scalar value to stack.
-                 * This is required so that we can limit the stream length,
-                 * which in turn allows to use same callback for packed and
-                 * not-packed fields. */
-                pb_istream_t substream;
-                uint8_t buffer[10];
-                size_t size = sizeof(buffer);
-                
-                if (!read_raw_value(stream, wire_type, buffer, &size))
-                    return false;
-                substream = pb_istream_from_buffer(buffer, size);
-                
-                return pCallback->funcs.decode(&substream, iter->current, pCallback->arg);
-            }
+            if (!pCallback->funcs.decode(&substream, iter->current, pCallback->arg))
+                PB_RETURN_ERROR(stream, "callback failed");
         }
         
+        pb_close_string_substream(stream, &substream);
+        return true;
+    }
+    else
+    {
+        /* Copy the single scalar value to stack.
+         * This is required so that we can limit the stream length,
+         * which in turn allows to use same callback for packed and
+         * not-packed fields. */
+        pb_istream_t substream;
+        uint8_t buffer[10];
+        size_t size = sizeof(buffer);
+        
+        if (!read_raw_value(stream, wire_type, buffer, &size))
+            return false;
+        substream = pb_istream_from_buffer(buffer, size);
+        
+        return pCallback->funcs.decode(&substream, iter->current, pCallback->arg);
+    }
+}
+
+static bool checkreturn decode_field(pb_istream_t *stream, pb_wire_type_t wire_type, pb_field_iterator_t *iter)
+{
+    switch (PB_ATYPE(iter->current->type))
+    {
+        case PB_ATYPE_STATIC:
+            return decode_static_field(stream, wire_type, iter);
+        
+        case PB_ATYPE_CALLBACK:
+            return decode_callback_field(stream, wire_type, iter);
+        
         default:
             PB_RETURN_ERROR(stream, "invalid field type");
     }
@@ -451,37 +473,43 @@ static void pb_message_set_to_defaults(const pb_field_t fields[], void *dest_str
     /* Initialize size/has fields and apply default values */
     do
     {
+        pb_type_t type;
+        type = iter.current->type;
+    
         if (iter.current->tag == 0)
             continue;
         
-        /* Initialize the size field for optional/repeated fields to 0. */
-        if (PB_HTYPE(iter.current->type) == PB_HTYPE_OPTIONAL)
-        {
-            *(bool*)iter.pSize = false;
-        }
-        else if (PB_HTYPE(iter.current->type) == PB_HTYPE_REPEATED)
+        if (PB_ATYPE(type) == PB_ATYPE_STATIC)
         {
-            *(size_t*)iter.pSize = 0;
-            continue; /* Array is empty, no need to initialize contents */
+            /* Initialize the size field for optional/repeated fields to 0. */
+            if (PB_HTYPE(type) == PB_HTYPE_OPTIONAL)
+            {
+                *(bool*)iter.pSize = false;
+            }
+            else if (PB_HTYPE(type) == PB_HTYPE_REPEATED)
+            {
+                *(size_t*)iter.pSize = 0;
+                continue; /* Array is empty, no need to initialize contents */
+            }
+            
+            /* Initialize field contents to default value */
+            if (PB_LTYPE(iter.current->type) == PB_LTYPE_SUBMESSAGE)
+            {
+                pb_message_set_to_defaults((const pb_field_t *) iter.current->ptr, iter.pData);
+            }
+            else if (iter.current->ptr != NULL)
+            {
+                memcpy(iter.pData, iter.current->ptr, iter.current->data_size);
+            }
+            else
+            {
+                memset(iter.pData, 0, iter.current->data_size);
+            }
         }
-        
-        /* Initialize field contents to default value */
-        if (PB_HTYPE(iter.current->type) == PB_HTYPE_CALLBACK)
+        else if (PB_ATYPE(type) == PB_ATYPE_CALLBACK)
         {
             continue; /* Don't overwrite callback */
         }
-        else if (PB_LTYPE(iter.current->type) == PB_LTYPE_SUBMESSAGE)
-        {
-            pb_message_set_to_defaults((const pb_field_t *) iter.current->ptr, iter.pData);
-        }
-        else if (iter.current->ptr != NULL)
-        {
-            memcpy(iter.pData, iter.current->ptr, iter.current->data_size);
-        }
-        else
-        {
-            memset(iter.pData, 0, iter.current->data_size);
-        }
     } while (pb_field_next(&iter));
 }
 
diff --git a/pb_encode.c b/pb_encode.c
index 85d65782..fbeeacfc 100644
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -153,58 +153,89 @@ static bool checkreturn encode_array(pb_ostream_t *stream, const pb_field_t *fie
     return true;
 }
 
+bool checkreturn encode_static_field(pb_ostream_t *stream, const pb_field_t *field, const void *pData)
+{
+    pb_encoder_t func;
+    const void *pSize;
+    
+    func = PB_ENCODERS[PB_LTYPE(field->type)];
+    pSize = (const char*)pData + field->size_offset;
+    
+    switch (PB_HTYPE(field->type))
+    {
+        case PB_HTYPE_REQUIRED:
+            if (!pb_encode_tag_for_field(stream, field))
+                return false;
+            if (!func(stream, field, pData))
+                return false;
+            break;
+        
+        case PB_HTYPE_OPTIONAL:
+            if (*(const bool*)pSize)
+            {
+                if (!pb_encode_tag_for_field(stream, field))
+                    return false;
+            
+                if (!func(stream, field, pData))
+                    return false;
+            }
+            break;
+        
+        case PB_HTYPE_REPEATED:
+            if (!encode_array(stream, field, pData, *(const size_t*)pSize, func))
+                return false;
+            break;
+        
+        default:
+            return false;
+    }
+    
+    return true;
+}
+
+bool checkreturn encode_callback_field(pb_ostream_t *stream, const pb_field_t *field, const void *pData)
+{
+    const pb_callback_t *callback = (const pb_callback_t*)pData;
+    if (callback->funcs.encode != NULL)
+    {
+        if (!callback->funcs.encode(stream, field, callback->arg))
+            return false;
+    }
+    return true;
+}
+
 bool checkreturn pb_encode(pb_ostream_t *stream, const pb_field_t fields[], const void *src_struct)
 {
     const pb_field_t *field = fields;
     const void *pData = src_struct;
-    const void *pSize;
     size_t prev_size = 0;
     
     while (field->tag != 0)
     {
-        pb_encoder_t func = PB_ENCODERS[PB_LTYPE(field->type)];
         pData = (const char*)pData + prev_size + field->data_offset;
-        pSize = (const char*)pData + field->size_offset;
-        
         prev_size = field->data_size;
-        if (PB_HTYPE(field->type) == PB_HTYPE_REPEATED)
+        
+        /* Special case for static arrays */
+        if (PB_ATYPE(field->type) == PB_ATYPE_STATIC &&
+            PB_HTYPE(field->type) == PB_HTYPE_REPEATED)
+        {
             prev_size *= field->array_size;
+        }
                 
-        switch (PB_HTYPE(field->type))
+        switch (PB_ATYPE(field->type))
         {
-            case PB_HTYPE_REQUIRED:
-                if (!pb_encode_tag_for_field(stream, field))
+            case PB_ATYPE_STATIC:
+                if (!encode_static_field(stream, field, pData))
                     return false;
-                if (!func(stream, field, pData))
-                    return false;
-                break;
-            
-            case PB_HTYPE_OPTIONAL:
-                if (*(const bool*)pSize)
-                {
-                    if (!pb_encode_tag_for_field(stream, field))
-                        return false;
-                
-                    if (!func(stream, field, pData))
-                        return false;
-                }
                 break;
             
-            case PB_HTYPE_REPEATED:
-                if (!encode_array(stream, field, pData, *(const size_t*)pSize, func))
+            case PB_ATYPE_CALLBACK:
+                if (!encode_callback_field(stream, field, pData))
                     return false;
                 break;
             
-            case PB_HTYPE_CALLBACK:
-            {
-                const pb_callback_t *callback = (const pb_callback_t*)pData;
-                if (callback->funcs.encode != NULL)
-                {
-                    if (!callback->funcs.encode(stream, field, callback->arg))
-                        return false;
-                }
-                break;
-            }
+            default:
+                return false;
         }
     
         field++;