Fixed a bug related to submessage encoding into memory buffer.

Stream state was not copied back from substream in pb_enc_submessage,
which caused garbage output if the stream callback modified the state.

Expanded tests to cover this problem.

Thanks to Paweł Pery for debugging and reporting this problem.

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

diff --git a/pb_encode.c b/pb_encode.c
index 317e31e..995eb3d 100644 (file)
--- a/pb_encode.c
+++ b/pb_encode.c
@@ -376,6 +376,7 @@ bool checkreturn pb_enc_submessage(pb_ostream_t *stream, const pb_field_t *field
     status = pb_encode(&substream, (pb_field_t*)field->ptr, src);
     
     stream->bytes_written += substream.bytes_written;
+    stream->state = substream.state;
     
     if (substream.bytes_written != size)
         return false;

diff --git a/tests/Makefile b/tests/Makefile
index 2d5ee50..a942b78 100644 (file)
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -19,6 +19,7 @@ pb_decode.o: ../pb_decode.c $(DEPS)
 
 test_decode1: test_decode1.o pb_decode.o person.pb.o
 test_encode1: test_encode1.o pb_encode.o person.pb.o
+test_encode2: test_encode2.o pb_encode.o person.pb.o
 test_decode_callbacks: test_decode_callbacks.o pb_decode.o callbacks.pb.o
 test_encode_callbacks: test_encode_callbacks.o pb_encode.o callbacks.pb.o
 decode_unittests: decode_unittests.o pb_decode.o unittestproto.pb.o
@@ -37,7 +38,7 @@ coverage: run_unittests
        gcov pb_encode.gcda
        gcov pb_decode.gcda
 
-run_unittests: decode_unittests encode_unittests test_encode1 test_decode1 test_encode_callbacks test_decode_callbacks
+run_unittests: decode_unittests encode_unittests test_encode1 test_encode2 test_decode1 test_encode_callbacks test_decode_callbacks
        rm -f *.gcda
        
        ./decode_unittests > /dev/null
@@ -45,6 +46,9 @@ run_unittests: decode_unittests encode_unittests test_encode1 test_decode1 test_
        
        [ "`./test_encode1 | ./test_decode1`" = \
        "`./test_encode1 | protoc --decode=Person -I. -I../generator -I/usr/include person.proto`" ]
+
+       [ "`./test_encode2 | ./test_decode1`" = \
+       "`./test_encode2 | protoc --decode=Person -I. -I../generator -I/usr/include person.proto`" ]
        
        [ "`./test_encode_callbacks | ./test_decode_callbacks`" = \
        "`./test_encode_callbacks | protoc --decode=TestMessage callbacks.proto`" ]

diff --git a/tests/test_decode1.c b/tests/test_decode1.c
index b412ea8..3b72f66 100644 (file)
--- a/tests/test_decode1.c
+++ b/tests/test_decode1.c
@@ -34,19 +34,22 @@ bool print_person(pb_istream_t *stream)
         printf("phone {\n");
         printf("  number: \"%s\"\n", phone->number);
         
-        switch (phone->type)
+        if (phone->has_type)
         {
-            case Person_PhoneType_WORK:
-                printf("  type: WORK\n");
-                break;
-            
-            case Person_PhoneType_HOME:
-                printf("  type: HOME\n");
-                break;
-            
-            case Person_PhoneType_MOBILE:
-                printf("  type: MOBILE\n");
-                break;
+            switch (phone->type)
+            {
+                case Person_PhoneType_WORK:
+                    printf("  type: WORK\n");
+                    break;
+                
+                case Person_PhoneType_HOME:
+                    printf("  type: HOME\n");
+                    break;
+                
+                case Person_PhoneType_MOBILE:
+                    printf("  type: MOBILE\n");
+                    break;
+            }
         }
         printf("}\n");
     }

diff --git a/tests/test_encode1.c b/tests/test_encode1.c
index f46e60a..c5131e4 100644 (file)
--- a/tests/test_encode1.c
+++ b/tests/test_encode1.c
@@ -1,31 +1,32 @@
 /* A very simple encoding test case using person.proto.
- * Just puts constant data in the fields and writes the
- * data to stdout.
+ * Just puts constant data in the fields and encodes into
+ * buffer, which is then written to stdout.
  */
 
 #include <stdio.h>
 #include <pb_encode.h>
 #include "person.pb.h"
 
-/* This binds the pb_ostream_t into the stdout stream */
-bool streamcallback(pb_ostream_t *stream, const uint8_t *buf, size_t count)
-{
-    FILE *file = (FILE*) stream->state;
-    return fwrite(buf, 1, count, file) == count;
-}
-
 int main()
 {
     /* Initialize the structure with constants */
     Person person = {"Test Person 99", 99, true, "test@person.com",
-        1, {{"555-12345678", true, Person_PhoneType_MOBILE}}};
-    
-    /* Prepare the stream, output goes directly to stdout */
-    pb_ostream_t stream = {&streamcallback, stdout, SIZE_MAX, 0};
+        3, {{"555-12345678", true, Person_PhoneType_MOBILE},
+            {"99-2342", false, 0},
+            {"1234-5678", true, Person_PhoneType_WORK},
+        }};
+
+    uint8_t buffer[512];
+    pb_ostream_t stream = pb_ostream_from_buffer(buffer, sizeof(buffer));
     
     /* Now encode it and check if we succeeded. */
     if (pb_encode(&stream, Person_fields, &person))
+    {
+        fwrite(buffer, stream.bytes_written, 1, stdout);
         return 0; /* Success */
+    }
     else
+    {
         return 1; /* Failure */
+    }
 }

diff --git a/tests/test_encode2.c b/tests/test_encode2.c
new file mode 100644 (file)
index 0000000..b1105ce
--- /dev/null
+++ b/tests/test_encode2.c
@@ -0,0 +1,32 @@
+/* Same as test_encode1.c, except writes directly to stdout.
+ */
+
+#include <stdio.h>
+#include <pb_encode.h>
+#include "person.pb.h"
+
+/* This binds the pb_ostream_t into the stdout stream */
+bool streamcallback(pb_ostream_t *stream, const uint8_t *buf, size_t count)
+{
+    FILE *file = (FILE*) stream->state;
+    return fwrite(buf, 1, count, file) == count;
+}
+
+int main()
+{
+    /* Initialize the structure with constants */
+    Person person = {"Test Person 99", 99, true, "test@person.com",
+        3, {{"555-12345678", true, Person_PhoneType_MOBILE},
+            {"99-2342", false, 0},
+            {"1234-5678", true, Person_PhoneType_WORK},
+        }};
+    
+    /* Prepare the stream, output goes directly to stdout */
+    pb_ostream_t stream = {&streamcallback, stdout, SIZE_MAX, 0};
+    
+    /* Now encode it and check if we succeeded. */
+    if (pb_encode(&stream, Person_fields, &person))
+        return 0; /* Success */
+    else
+        return 1; /* Failure */
+}