Fixed a bunch of bugs related to callback fields.

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

6 files changed, 117 insertions, 40 deletions

diff --git a/tests/Makefile b/tests/Makefile
index 30bce647..2d5ee506 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -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
diff --git a/tests/callbacks.proto b/tests/callbacks.proto
index 7bc79002..8beeaabf 100644
--- a/tests/callbacks.proto
+++ b/tests/callbacks.proto
@@ -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;
 }
 
diff --git a/tests/decode_unittests.c b/tests/decode_unittests.c
index 006ad900..ab12ac33 100644
--- a/tests/decode_unittests.c
+++ b/tests/decode_unittests.c
@@ -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))
     }
     
     {
diff --git a/tests/encode_unittests.c b/tests/encode_unittests.c
index 926a2545..166e6e83 100644
--- a/tests/encode_unittests.c
+++ b/tests/encode_unittests.c
@@ -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"))
     }
     
     {
diff --git a/tests/test_decode_callbacks.c b/tests/test_decode_callbacks.c
index 1c8d43a5..6c0072de 100644
--- a/tests/test_decode_callbacks.c
+++ b/tests/test_decode_callbacks.c
@@ -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;
diff --git a/tests/test_encode_callbacks.c b/tests/test_encode_callbacks.c
index da2ee28e..f0a046de 100644
--- a/tests/test_encode_callbacks.c
+++ b/tests/test_encode_callbacks.c
@@ -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;