Fixed a bunch of bugs related to callback fields.
authorPetteri Aimonen <jpa@npb.mail.kapsi.fi>
Tue, 13 Sep 2011 16:14:08 +0000 (16:14 +0000)
committerPetteri Aimonen <jpa@npb.mail.kapsi.fi>
Tue, 13 Sep 2011 16:14:08 +0000 (16:14 +0000)
Most importantly, callback fields in submessages were being overwritten with garbage, causing segfaults.

Additionally, converted PB_LTYPE_FIXED to PB_LTYPE_FIXED32 and PB_LTYPE_FIXED64. This makes the interface
a bit easier to use, and in addition runs faster.

git-svn-id: https://svn.kapsi.fi/jpa/nanopb@975 e3a754e5-d11d-0410-8d38-ebb782a927b9

13 files changed:
docs/reference.rst
generator/nanopb_generator.py
pb.h
pb_decode.c
pb_decode.h
pb_encode.c
pb_encode.h
tests/Makefile
tests/callbacks.proto
tests/decode_unittests.c
tests/encode_unittests.c
tests/test_decode_callbacks.c
tests/test_encode_callbacks.c

index 55ade5e..71b934c 100644 (file)
@@ -62,7 +62,7 @@ Describes a single structure field with memory position in relation to others. T
 :type:          LTYPE and HTYPE of the field.
 :data_offset:   Offset of field data, relative to the end of the previous field.
 :size_offset:   Offset of *bool* flag for optional fields or *size_t* count for arrays, relative to field data.
-:data_size:     Size of a single data entry, in bytes. For PB_LTYPE_BYTES, the size of the byte array inside the containing structure.
+:data_size:     Size of a single data entry, in bytes. For PB_LTYPE_BYTES, the size of the byte array inside the containing structure. For PB_HTYPE_CALLBACK, size of the C data type if known.
 :array_size:    Maximum number of entries in an array, if it is an array type.
 :ptr:           Pointer to default value for optional fields, or to submessage description for PB_LTYPE_SUBMESSAGE.
 
@@ -190,9 +190,9 @@ Wire type mapping is as follows:
 LTYPEs                    Wire type
 ========================= ============
 VARINT, SVARINT           PB_WT_VARINT
-FIXED with data_size == 8 PB_WT_64BIT  
+FIXED64                   PB_WT_64BIT  
 STRING, BYTES, SUBMESSAGE PB_WT_STRING 
-FIXED with data_size == 4 PB_WT_32BIT
+FIXED32                   PB_WT_32BIT
 ========================= ============
 
 pb_encode_string
@@ -214,7 +214,7 @@ Writes the length of a string as varint and then contents of the string. Used fo
 
     Each field encoder only encodes the contents of the field. The tag must be encoded separately with `pb_encode_tag_for_field`_.
 
-    You can use the field encoders from your callbacks.
+    You can use the field encoders from your callbacks. Just be aware that the pb_field_t passed to the callback is not directly compatible with most of the encoders. Instead, you must create a new pb_field_t structure and set the data_size according to the data type you pass to *src.
 
 pb_enc_varint
 -------------
@@ -237,15 +237,26 @@ Field encoder for PB_LTYPE_SVARINT. Similar to `pb_enc_varint`_, except first zi
 
 The number is considered negative if the high-order bit of the value is set. On big endian computers, it is the highest bit of *\*src*. On little endian computers, it is the highest bit of *\*(src + field->data_size - 1)*.
 
-pb_enc_fixed
-------------
-Field encoder for PB_LTYPE_FIXED. Writes the data in little endian order. On big endian computers, reverses the order of bytes. ::
+pb_enc_fixed32
+--------------
+Field encoder for PB_LTYPE_FIXED32. Writes the data in little endian order. On big endian computers, reverses the order of bytes. ::
 
-    bool pb_enc_fixed(pb_ostream_t *stream, const pb_field_t *field, const void *src);
+    bool pb_enc_fixed32(pb_ostream_t *stream, const pb_field_t *field, const void *src);
 
-(parameters are the same as for `pb_enc_varint`_)
+:stream:        Output stream to write to.
+:field:         Not used.
+:src:           Pointer to start of the field data.
+:returns:       True on success, false on IO error.
+
+pb_enc_fixed64
+--------------
+Field encoder for PB_LTYPE_FIXED64. Writes the data in little endian order. On big endian computers, reverses the order of bytes. ::
 
-The same function is used for both integers, floats and doubles. This break encoding of double values on architectures where they are mixed endian (primarily some arm processors with hardware FPU).
+    bool pb_enc_fixed64(pb_ostream_t *stream, const pb_field_t *field, const void *src);
+
+(parameters are the same as for `pb_enc_fixed32`_)
+
+The same function is used for both integers and doubles. This breaks encoding of double values on architectures where they are mixed endian (primarily some arm processors with hardware FPU).
 
 pb_enc_bytes
 ------------
@@ -365,7 +376,7 @@ Because of memory concerns, the detection of missing required fields is not perf
 
     Each field decoder reads and decodes a single value. For arrays, the decoder is called repeatedly.
 
-    You can use the decoders from your callbacks.
+    You can use the decoders from your callbacks. Just be aware that the pb_field_t passed to the callback is not directly compatible with most of the field decoders. Instead, you must create a new pb_field_t structure and set the data_size according to the data type you pass to *dest.
 
 pb_dec_varint
 -------------
@@ -388,18 +399,29 @@ Field decoder for PB_LTYPE_SVARINT. Similar to `pb_dec_varint`_, except that it
 
 (parameters are the same as `pb_dec_varint`_)
 
-pb_dec_fixed
-------------
-Field decoder for PB_LTYPE_FIXED. ::
+pb_dec_fixed32
+--------------
+Field decoder for PB_LTYPE_FIXED32. ::
 
     bool pb_dec_fixed(pb_istream_t *stream, const pb_field_t *field, void *dest);
 
-(parameters are the same as `pb_dec_varint`_)
+:stream:        Input stream to read from. 1-10 bytes will be read.
+:field:         Not used.
+:dest:          Pointer to destination integer. Must have size of *field->data_size* bytes.
+:returns:       True on success, false on IO errors or if `pb_decode_varint`_ fails.
 
-This function reads *field->data_size* bytes from the input stream.
+This function reads 4 bytes from the input stream.
 On big endian architectures, it then reverses the order of the bytes.
 Finally, it writes the bytes to *dest*.
 
+pb_dec_fixed64
+--------------
+Field decoder for PB_LTYPE_FIXED64. ::
+
+    bool pb_dec_fixed(pb_istream_t *stream, const pb_field_t *field, void *dest);
+
+Same as `pb_dec_fixed32`_, except this reads 8 bytes.
+
 pb_dec_bytes
 ------------
 Field decoder for PB_LTYPE_BYTES. Reads a length-prefixed block of bytes. ::
index 22331f0..4d5aaac 100644 (file)
@@ -8,10 +8,10 @@ import os.path
 FieldD = descriptor.FieldDescriptorProto
 datatypes = {
     FieldD.TYPE_BOOL: ('bool', 'PB_LTYPE_VARINT'),
-    FieldD.TYPE_DOUBLE: ('double', 'PB_LTYPE_FIXED'),
-    FieldD.TYPE_FIXED32: ('uint32_t', 'PB_LTYPE_FIXED'),
-    FieldD.TYPE_FIXED64: ('uint64_t', 'PB_LTYPE_FIXED'),
-    FieldD.TYPE_FLOAT: ('float', 'PB_LTYPE_FIXED'),
+    FieldD.TYPE_DOUBLE: ('double', 'PB_LTYPE_FIXED64'),
+    FieldD.TYPE_FIXED32: ('uint32_t', 'PB_LTYPE_FIXED32'),
+    FieldD.TYPE_FIXED64: ('uint64_t', 'PB_LTYPE_FIXED64'),
+    FieldD.TYPE_FLOAT: ('float', 'PB_LTYPE_FIXED32'),
     FieldD.TYPE_INT32: ('int32_t', 'PB_LTYPE_VARINT'),
     FieldD.TYPE_INT64: ('int64_t', 'PB_LTYPE_VARINT'),
     FieldD.TYPE_SFIXED32: ('int32_t', 'PB_LTYPE_FIXED'),
diff --git a/pb.h b/pb.h
index 2c6a252..cd9c75d 100644 (file)
--- a/pb.h
+++ b/pb.h
@@ -35,25 +35,26 @@ typedef enum {
     /* Numeric types */
     PB_LTYPE_VARINT = 0x00, /* int32, uint32, int64, uint64, bool, enum */
     PB_LTYPE_SVARINT = 0x01, /* sint32, sint64 */
-    PB_LTYPE_FIXED = 0x02, /* fixed32, sfixed32, fixed64, sfixed64, float, double */
+    PB_LTYPE_FIXED32 = 0x02, /* fixed32, sfixed32, float */
+    PB_LTYPE_FIXED64 = 0x03, /* fixed64, sfixed64, double */
     
     /* Marker for last packable field type. */
-    PB_LTYPE_LAST_PACKABLE = 0x02,
+    PB_LTYPE_LAST_PACKABLE = 0x03,
     
     /* Byte array with pre-allocated buffer.
      * data_size is the length of the allocated PB_BYTES_ARRAY structure. */
-    PB_LTYPE_BYTES = 0x03,
+    PB_LTYPE_BYTES = 0x04,
     
     /* String with pre-allocated buffer.
      * data_size is the maximum length. */
-    PB_LTYPE_STRING = 0x04,
+    PB_LTYPE_STRING = 0x05,
     
     /* Submessage
      * submsg_fields is pointer to field descriptions */
-    PB_LTYPE_SUBMESSAGE = 0x05,
+    PB_LTYPE_SUBMESSAGE = 0x06,
     
     /* Number of declared LTYPES */
-    PB_LTYPES_COUNT = 6,
+    PB_LTYPES_COUNT = 7,
     
     /******************
      * Modifier flags *
index 6c6b1d9..453b1cc 100644 (file)
@@ -23,7 +23,8 @@ typedef bool (*pb_decoder_t)(pb_istream_t *stream, const pb_field_t *field, void
 static const pb_decoder_t PB_DECODERS[PB_LTYPES_COUNT] = {
     &pb_dec_varint,
     &pb_dec_svarint,
-    &pb_dec_fixed,
+    &pb_dec_fixed32,
+    &pb_dec_fixed64,
     
     &pb_dec_bytes,
     &pb_dec_string,
@@ -163,7 +164,10 @@ static bool checkreturn read_raw_value(pb_istream_t *stream, pb_wire_type_t wire
     }
 }
 
-/* Decode string length from stream and return a substream with limited length */
+/* Decode string length from stream and return a substream with limited length.
+ * Before disposing the substream, remember to copy the substream->state back
+ * to stream->state.
+ */
 static bool checkreturn make_string_substream(pb_istream_t *stream, pb_istream_t *substream)
 {
     uint32_t size;
@@ -302,6 +306,8 @@ static bool checkreturn decode_field(pb_istream_t *stream, int wire_type, pb_fie
                     if (!pCallback->funcs.decode(&substream, iter->current, pCallback->arg))
                         return false;
                 }
+                
+                stream->state = substream.state;
                 return true;
             }
             else
@@ -327,16 +333,10 @@ static bool checkreturn decode_field(pb_istream_t *stream, int wire_type, pb_fie
     }
 }
 
-/*********************
- * Decode all fields *
- *********************/
-
-bool checkreturn pb_decode(pb_istream_t *stream, const pb_field_t fields[], void *dest_struct)
+/* Initialize message fields to default values, recursively */
+static void pb_message_set_to_defaults(const pb_field_t fields[], void *dest_struct)
 {
-    uint32_t fields_seen = 0; /* Used to check for required fields */
     pb_field_iterator_t iter;
-    int i;
-    
     pb_field_init(&iter, fields, dest_struct);
     
     /* Initialize size/has fields and apply default values */
@@ -345,25 +345,50 @@ bool checkreturn pb_decode(pb_istream_t *stream, const pb_field_t fields[], void
         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;
-            
-            /* Initialize to default value */
-            if (iter.current->ptr != NULL)
-                memcpy(iter.pData, iter.current->ptr, iter.current->data_size);
-            else
-                memset(iter.pData, 0, iter.current->data_size);
         }
         else if (PB_HTYPE(iter.current->type) == PB_HTYPE_ARRAY)
         {
             *(size_t*)iter.pSize = 0;
+            continue; /* Array is empty, no need to initialize contents */
+        }
+        
+        /* Initialize field contents to default value */
+        if (PB_HTYPE(iter.current->type) == PB_HTYPE_CALLBACK)
+        {
+            continue; /* Don't overwrite callback */
+        }
+        else if (PB_LTYPE(iter.current->type) == PB_LTYPE_SUBMESSAGE)
+        {
+            pb_message_set_to_defaults(iter.current->ptr, iter.pData);
+        }
+        else if (iter.current->ptr != NULL)
+        {
+            memcpy(iter.pData, iter.current->ptr, iter.current->data_size);
         }
-        else if (PB_HTYPE(iter.current->type) == PB_HTYPE_REQUIRED)
+        else
         {
             memset(iter.pData, 0, iter.current->data_size);
         }
     } while (pb_field_next(&iter));
+}
+
+/*********************
+ * Decode all fields *
+ *********************/
+
+bool checkreturn pb_decode(pb_istream_t *stream, const pb_field_t fields[], void *dest_struct)
+{
+    uint32_t fields_seen = 0; /* Used to check for required fields */
+    pb_field_iterator_t iter;
+    int i;
+    
+    pb_message_set_to_defaults(fields, dest_struct);
+    
+    pb_field_init(&iter, fields, dest_struct);
     
     while (stream->bytes_left)
     {
@@ -443,17 +468,30 @@ bool checkreturn pb_dec_svarint(pb_istream_t *stream, const pb_field_t *field, v
     return status;
 }
 
-bool checkreturn pb_dec_fixed(pb_istream_t *stream, const pb_field_t *field, void *dest)
+bool checkreturn pb_dec_fixed32(pb_istream_t *stream, const pb_field_t *field, void *dest)
+{
+#ifdef __BIG_ENDIAN__
+    uint8_t bytes[4] = {0};
+    bool status = pb_read(stream, bytes, 4);
+    uint8_t bebytes[4] = {bytes[3], bytes[2], bytes[1], bytes[0]};
+    memcpy(dest, bebytes, 4);
+    return status;
+#else
+    return pb_read(stream, (uint8_t*)dest, 4);
+#endif
+}
+
+bool checkreturn pb_dec_fixed64(pb_istream_t *stream, const pb_field_t *field, void *dest)
 {
 #ifdef __BIG_ENDIAN__
     uint8_t bytes[8] = {0};
-    bool status = pb_read(stream, bytes, field->data_size);
+    bool status = pb_read(stream, bytes, 8);
     uint8_t bebytes[8] = {bytes[7], bytes[6], bytes[5], bytes[4], 
                           bytes[3], bytes[2], bytes[1], bytes[0]};
-    endian_copy(dest, lebytes, field->data_size, 8);
+    memcpy(dest, bebytes, 8);
     return status;
 #else
-    return pb_read(stream, (uint8_t*)dest, field->data_size);
+    return pb_read(stream, (uint8_t*)dest, 8);
 #endif
 }
 
@@ -489,6 +527,7 @@ bool checkreturn pb_dec_string(pb_istream_t *stream, const pb_field_t *field, vo
 
 bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t *field, void *dest)
 {
+    bool status;
     pb_istream_t substream;
     
     if (!make_string_substream(stream, &substream))
@@ -497,5 +536,7 @@ bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t *field
     if (field->ptr == NULL)
         return false;
     
-    return pb_decode(&substream, (pb_field_t*)field->ptr, dest);
+    status = pb_decode(&substream, (pb_field_t*)field->ptr, dest);
+    stream->state = substream.state;
+    return status;
 }
index 2d4e586..f12b190 100644 (file)
@@ -61,7 +61,8 @@ bool pb_skip_string(pb_istream_t *stream);
 
 bool pb_dec_varint(pb_istream_t *stream, const pb_field_t *field, void *dest);
 bool pb_dec_svarint(pb_istream_t *stream, const pb_field_t *field, void *dest);
-bool pb_dec_fixed(pb_istream_t *stream, const pb_field_t *field, void *dest);
+bool pb_dec_fixed32(pb_istream_t *stream, const pb_field_t *field, void *dest);
+bool pb_dec_fixed64(pb_istream_t *stream, const pb_field_t *field, void *dest);
 
 bool pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest);
 bool pb_dec_string(pb_istream_t *stream, const pb_field_t *field, void *dest);
index e83e068..58e03a6 100644 (file)
@@ -23,7 +23,8 @@ typedef bool (*pb_encoder_t)(pb_ostream_t *stream, const pb_field_t *field, cons
 static const pb_encoder_t PB_ENCODERS[PB_LTYPES_COUNT] = {
     &pb_enc_varint,
     &pb_enc_svarint,
-    &pb_enc_fixed,
+    &pb_enc_fixed32,
+    &pb_enc_fixed64,
     
     &pb_enc_bytes,
     &pb_enc_string,
@@ -87,9 +88,13 @@ static bool checkreturn encode_array(pb_ostream_t *stream, const pb_field_t *fie
             return false;
         
         /* Determine the total size of packed array. */
-        if (PB_LTYPE(field->type) == PB_LTYPE_FIXED)
+        if (PB_LTYPE(field->type) == PB_LTYPE_FIXED32)
         {
-            size = field->data_size * count;
+            size = 4 * count;
+        }
+        else if (PB_LTYPE(field->type) == PB_LTYPE_FIXED64)
+        {
+            size = 8 * count;
         }
         else
         {
@@ -232,13 +237,12 @@ bool checkreturn pb_encode_tag_for_field(pb_ostream_t *stream, const pb_field_t
             wiretype = PB_WT_VARINT;
             break;
         
-        case PB_LTYPE_FIXED:
-            if (field->data_size == 4)
-                wiretype = PB_WT_32BIT;
-            else if (field->data_size == 8)
-                wiretype = PB_WT_64BIT;
-            else
-                return false;
+        case PB_LTYPE_FIXED32:
+            wiretype = PB_WT_32BIT;
+            break;
+        
+        case PB_LTYPE_FIXED64:
+            wiretype = PB_WT_64BIT;
             break;
         
         case PB_LTYPE_BYTES:
@@ -304,16 +308,28 @@ bool checkreturn pb_enc_svarint(pb_ostream_t *stream, const pb_field_t *field, c
     return pb_encode_varint(stream, zigzagged);
 }
 
-bool checkreturn pb_enc_fixed(pb_ostream_t *stream, const pb_field_t *field, const void *src)
+bool checkreturn pb_enc_fixed64(pb_ostream_t *stream, const pb_field_t *field, const void *src)
 {
     #ifdef __BIG_ENDIAN__
     uint8_t bytes[8] = {0};
-    endian_copy(bytes, src, sizeof(bytes), field->data_size);
+    memcpy(bytes, src, 8);
     uint8_t lebytes[8] = {bytes[7], bytes[6], bytes[5], bytes[4], 
                           bytes[3], bytes[2], bytes[1], bytes[0]};
-    return pb_write(stream, lebytes, field->data_size);
+    return pb_write(stream, lebytes, 8);
+    #else
+    return pb_write(stream, (uint8_t*)src, 8);
+    #endif
+}
+
+bool checkreturn pb_enc_fixed32(pb_ostream_t *stream, const pb_field_t *field, const void *src)
+{
+    #ifdef __BIG_ENDIAN__
+    uint8_t bytes[4] = {0};
+    memcpy(bytes, src, 4);
+    uint8_t lebytes[4] = {bytes[3], bytes[2], bytes[1], bytes[0]};
+    return pb_write(stream, lebytes, 4);
     #else
-    return pb_write(stream, (uint8_t*)src, field->data_size);
+    return pb_write(stream, (uint8_t*)src, 4);
     #endif
 }
 
index b341602..864a48b 100644 (file)
@@ -62,7 +62,8 @@ bool pb_encode_string(pb_ostream_t *stream, const uint8_t *buffer, size_t size);
 
 bool pb_enc_varint(pb_ostream_t *stream, const pb_field_t *field, const void *src);
 bool pb_enc_svarint(pb_ostream_t *stream, const pb_field_t *field, const void *src);
-bool pb_enc_fixed(pb_ostream_t *stream, const pb_field_t *field, const void *src);
+bool pb_enc_fixed32(pb_ostream_t *stream, const pb_field_t *field, const void *src);
+bool pb_enc_fixed64(pb_ostream_t *stream, const pb_field_t *field, const void *src);
 
 bool pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src);
 bool pb_enc_string(pb_ostream_t *stream, const pb_field_t *field, const void *src);
index 30bce64..2d5ee50 100644 (file)
@@ -3,7 +3,7 @@ LDFLAGS=--coverage
 DEPS=../pb_decode.h ../pb_encode.h ../pb.h person.pb.h callbacks.pb.h unittests.h unittestproto.pb.h
 TESTS=test_decode1 test_encode1 decode_unittests encode_unittests
 
-all: $(TESTS) run_unittests breakpoints
+all: breakpoints $(TESTS) run_unittests
 
 clean:
        rm -f $(TESTS) person.pb* *.o *.gcda *.gcno
@@ -27,7 +27,7 @@ encode_unittests: encode_unittests.o pb_encode.o unittestproto.pb.o
 %.pb: %.proto
        protoc -I. -I../generator -I/usr/include -o$@ $<
 
-%.pb.c %.pb.h: %.pb
+%.pb.c %.pb.h: %.pb ../generator/nanopb_generator.py
        python ../generator/nanopb_generator.py $<
 
 breakpoints: ../*.c *.c
index 7bc7900..8beeaab 100644 (file)
@@ -1,16 +1,15 @@
-/* Todo: write tests for the rest of these fields, currently only stringvalue
- * is tested.
- */
-
 message SubMessage {
-    optional int32 int32value = 1;
+    optional string stringvalue = 1;
+    repeated int32 int32value = 2;
+    repeated fixed32 fixed32value = 3;
+    repeated fixed64 fixed64value = 4;
 }
 
 message TestMessage {
     optional string stringvalue = 1;
-    optional int32 int32value = 2;
-    optional fixed32 fixed32value = 3;
-    optional fixed64 fixed64value = 4;
+    repeated int32 int32value = 2;
+    repeated fixed32 fixed32value = 3;
+    repeated fixed64 fixed64value = 4;
     optional SubMessage submsg = 5;
 }
 
index 006ad90..ab12ac3 100644 (file)
@@ -143,25 +143,25 @@ int main()
     
     {
         pb_istream_t s;
-        pb_field_t f = {1, PB_LTYPE_FIXED, 0, 0, 4, 0, 0};
+        pb_field_t f = {1, PB_LTYPE_FIXED32, 0, 0, 4, 0, 0};
         float d;
         
-        COMMENT("Test pb_dec_fixed using float (failures here may be caused by imperfect rounding)")
-        TEST((s = S("\x00\x00\x00\x00"), pb_dec_fixed(&s, &f, &d) && d == 0.0f))
-        TEST((s = S("\x00\x00\xc6\x42"), pb_dec_fixed(&s, &f, &d) && d == 99.0f))
-        TEST((s = S("\x4e\x61\x3c\xcb"), pb_dec_fixed(&s, &f, &d) && d == -12345678.0f))
-        TEST((s = S("\x00"), !pb_dec_fixed(&s, &f, &d) && d == -12345678.0f))
+        COMMENT("Test pb_dec_fixed32 using float (failures here may be caused by imperfect rounding)")
+        TEST((s = S("\x00\x00\x00\x00"), pb_dec_fixed32(&s, &f, &d) && d == 0.0f))
+        TEST((s = S("\x00\x00\xc6\x42"), pb_dec_fixed32(&s, &f, &d) && d == 99.0f))
+        TEST((s = S("\x4e\x61\x3c\xcb"), pb_dec_fixed32(&s, &f, &d) && d == -12345678.0f))
+        TEST((s = S("\x00"), !pb_dec_fixed32(&s, &f, &d) && d == -12345678.0f))
     }
     
     {
         pb_istream_t s;
-        pb_field_t f = {1, PB_LTYPE_FIXED, 0, 0, 8, 0, 0};
+        pb_field_t f = {1, PB_LTYPE_FIXED64, 0, 0, 8, 0, 0};
         double d;
         
-        COMMENT("Test pb_dec_fixed using double (failures here may be caused by imperfect rounding)")
-        TEST((s = S("\x00\x00\x00\x00\x00\x00\x00\x00"), pb_dec_fixed(&s, &f, &d) && d == 0.0))
-        TEST((s = S("\x00\x00\x00\x00\x00\xc0\x58\x40"), pb_dec_fixed(&s, &f, &d) && d == 99.0))
-        TEST((s = S("\x00\x00\x00\xc0\x29\x8c\x67\xc1"), pb_dec_fixed(&s, &f, &d) && d == -12345678.0f))
+        COMMENT("Test pb_dec_fixed64 using double (failures here may be caused by imperfect rounding)")
+        TEST((s = S("\x00\x00\x00\x00\x00\x00\x00\x00"), pb_dec_fixed64(&s, &f, &d) && d == 0.0))
+        TEST((s = S("\x00\x00\x00\x00\x00\xc0\x58\x40"), pb_dec_fixed64(&s, &f, &d) && d == 99.0))
+        TEST((s = S("\x00\x00\x00\xc0\x29\x8c\x67\xc1"), pb_dec_fixed64(&s, &f, &d) && d == -12345678.0f))
     }
     
     {
index 926a254..166e6e8 100644 (file)
@@ -99,15 +99,13 @@ int main()
         COMMENT("Test pb_encode_tag_for_field")
         TEST(WRITES(pb_encode_tag_for_field(&s, &field), "\x50"));
         
-        field.type = PB_LTYPE_FIXED;
-        field.data_size = 8;
+        field.type = PB_LTYPE_FIXED64;
         TEST(WRITES(pb_encode_tag_for_field(&s, &field), "\x51"));
         
         field.type = PB_LTYPE_STRING;
         TEST(WRITES(pb_encode_tag_for_field(&s, &field), "\x52"));
         
-        field.type = PB_LTYPE_FIXED;
-        field.data_size = 4;
+        field.type = PB_LTYPE_FIXED32;
         TEST(WRITES(pb_encode_tag_for_field(&s, &field), "\x55"));
     }
     
@@ -149,26 +147,24 @@ int main()
     {
         uint8_t buffer[30];
         pb_ostream_t s;
-        pb_field_t field = {1, PB_LTYPE_FIXED, 0, 0, sizeof(float)};
         float fvalue;
         double dvalue;
         
-        COMMENT("Test pb_enc_fixed using float")
+        COMMENT("Test pb_enc_fixed32 using float")
         fvalue = 0.0f;
-        TEST(WRITES(pb_enc_fixed(&s, &field, &fvalue), "\x00\x00\x00\x00"))
+        TEST(WRITES(pb_enc_fixed32(&s, NULL, &fvalue), "\x00\x00\x00\x00"))
         fvalue = 99.0f;
-        TEST(WRITES(pb_enc_fixed(&s, &field, &fvalue), "\x00\x00\xc6\x42"))
+        TEST(WRITES(pb_enc_fixed32(&s, NULL, &fvalue), "\x00\x00\xc6\x42"))
         fvalue = -12345678.0f;
-        TEST(WRITES(pb_enc_fixed(&s, &field, &fvalue), "\x4e\x61\x3c\xcb"))
+        TEST(WRITES(pb_enc_fixed32(&s, NULL, &fvalue), "\x4e\x61\x3c\xcb"))
     
-        COMMENT("Test pb_enc_fixed using double")
-        field.data_size = sizeof(double);
+        COMMENT("Test pb_enc_fixed64 using double")
         dvalue = 0.0;
-        TEST(WRITES(pb_enc_fixed(&s, &field, &dvalue), "\x00\x00\x00\x00\x00\x00\x00\x00"))
+        TEST(WRITES(pb_enc_fixed64(&s, NULL, &dvalue), "\x00\x00\x00\x00\x00\x00\x00\x00"))
         dvalue = 99.0;
-        TEST(WRITES(pb_enc_fixed(&s, &field, &dvalue), "\x00\x00\x00\x00\x00\xc0\x58\x40"))
+        TEST(WRITES(pb_enc_fixed64(&s, NULL, &dvalue), "\x00\x00\x00\x00\x00\xc0\x58\x40"))
         dvalue = -12345678.0;
-        TEST(WRITES(pb_enc_fixed(&s, &field, &dvalue), "\x00\x00\x00\xc0\x29\x8c\x67\xc1"))
+        TEST(WRITES(pb_enc_fixed64(&s, NULL, &dvalue), "\x00\x00\x00\xc0\x29\x8c\x67\xc1"))
     }
     
     {
index 1c8d43a..6c0072d 100644 (file)
@@ -8,17 +8,49 @@
 
 bool print_string(pb_istream_t *stream, const pb_field_t *field, void *arg)
 {
-    uint8_t buffer[1024];
+    uint8_t buffer[1024] = {0};
     
     /* We could read block-by-block to avoid the large buffer... */
-    if (stream->bytes_left > sizeof(buffer))
+    if (stream->bytes_left > sizeof(buffer) - 1)
         return false;
     
     if (!pb_read(stream, buffer, stream->bytes_left))
         return false;
     
-    /* Print the string, in format comparable with protoc --decode. */
-    printf("%s: \"%s\"\n", (char*)arg, buffer);
+    /* Print the string, in format comparable with protoc --decode.
+     * Format comes from the arg defined in main().
+     */
+    printf((char*)arg, buffer);
+    return true;
+}
+
+bool print_int32(pb_istream_t *stream, const pb_field_t *field, void *arg)
+{
+    uint64_t value;
+    if (!pb_decode_varint(stream, &value))
+        return false;
+    
+    printf((char*)arg, (int32_t)value);
+    return true;
+}
+
+bool print_fixed32(pb_istream_t *stream, const pb_field_t *field, void *arg)
+{
+    uint32_t value;
+    if (!pb_dec_fixed32(stream, NULL, &value))
+        return false;
+    
+    printf((char*)arg, value);
+    return true;
+}
+
+bool print_fixed64(pb_istream_t *stream, const pb_field_t *field, void *arg)
+{
+    uint64_t value;
+    if (!pb_dec_fixed64(stream, NULL, &value))
+        return false;
+    
+    printf((char*)arg, value);
     return true;
 }
 
@@ -34,8 +66,23 @@ int main()
      */
     TestMessage testmessage = {};
     
+    testmessage.submsg.stringvalue.funcs.decode = &print_string;
+    testmessage.submsg.stringvalue.arg = "submsg {\n  stringvalue: \"%s\"\n";
+    testmessage.submsg.int32value.funcs.decode = &print_int32;
+    testmessage.submsg.int32value.arg = "  int32value: %d\n";
+    testmessage.submsg.fixed32value.funcs.decode = &print_fixed32;
+    testmessage.submsg.fixed32value.arg = "  fixed32value: %d\n";
+    testmessage.submsg.fixed64value.funcs.decode = &print_fixed64;
+    testmessage.submsg.fixed64value.arg = "  fixed64value: %lld\n}\n";
+    
     testmessage.stringvalue.funcs.decode = &print_string;
-    testmessage.stringvalue.arg = "stringvalue";
+    testmessage.stringvalue.arg = "stringvalue: \"%s\"\n";
+    testmessage.int32value.funcs.decode = &print_int32;
+    testmessage.int32value.arg = "int32value: %d\n";
+    testmessage.fixed32value.funcs.decode = &print_fixed32;
+    testmessage.fixed32value.arg = "fixed32value: %d\n";
+    testmessage.fixed64value.funcs.decode = &print_fixed64;
+    testmessage.fixed64value.arg = "fixed64value: %lld\n";
     
     if (!pb_decode(&stream, TestMessage_fields, &testmessage))
         return 1;
index da2ee28..f0a046d 100644 (file)
@@ -15,6 +15,32 @@ bool encode_string(pb_ostream_t *stream, const pb_field_t *field, const void *ar
     return pb_encode_string(stream, (uint8_t*)str, strlen(str));
 }
 
+bool encode_int32(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+{
+    if (!pb_encode_tag_for_field(stream, field))
+        return false;
+    
+    return pb_encode_varint(stream, 42);
+}
+
+bool encode_fixed32(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+{
+    if (!pb_encode_tag_for_field(stream, field))
+        return false;
+    
+    uint32_t value = 42;
+    return pb_enc_fixed32(stream, field, &value);
+}
+
+bool encode_fixed64(pb_ostream_t *stream, const pb_field_t *field, const void *arg)
+{
+    if (!pb_encode_tag_for_field(stream, field))
+        return false;
+    
+    uint64_t value = 42;
+    return pb_enc_fixed64(stream, field, &value);
+}
+
 int main()
 {
     uint8_t buffer[1024];
@@ -22,6 +48,15 @@ int main()
     TestMessage testmessage = {};
     
     testmessage.stringvalue.funcs.encode = &encode_string;
+    testmessage.int32value.funcs.encode = &encode_int32;
+    testmessage.fixed32value.funcs.encode = &encode_fixed32;
+    testmessage.fixed64value.funcs.encode = &encode_fixed64;
+    
+    testmessage.has_submsg = true;
+    testmessage.submsg.stringvalue.funcs.encode = &encode_string;
+    testmessage.submsg.int32value.funcs.encode = &encode_int32;
+    testmessage.submsg.fixed32value.funcs.encode = &encode_fixed32;
+    testmessage.submsg.fixed64value.funcs.encode = &encode_fixed64;
     
     if (!pb_encode(&stream, TestMessage_fields, &testmessage))
         return 1;