From 1506450b119a504259983692cca4c8cd3daddaf1 Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Fri, 30 Dec 2011 08:43:50 +0000 Subject: [PATCH] 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. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- pb_encode.c | 1 + tests/Makefile | 6 +++++- tests/test_decode1.c | 27 +++++++++++++++------------ tests/test_encode1.c | 27 ++++++++++++++------------- tests/test_encode2.c | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 26 deletions(-) create mode 100644 tests/test_encode2.c diff --git a/pb_encode.c b/pb_encode.c index 317e31ef..995eb3de 100644 --- 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 2d5ee506..a942b786 100644 --- 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 b412ea85..3b72f664 100644 --- 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 f46e60a6..c5131e47 100644 --- 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 #include #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 index 00000000..b1105ce6 --- /dev/null +++ b/tests/test_encode2.c @@ -0,0 +1,32 @@ +/* Same as test_encode1.c, except writes directly to stdout. + */ + +#include +#include +#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 */ +} -- 2.16.6