unittests, change to PB_LTYPE_BYTES data size

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

6 files changed, 77 insertions, 18 deletions

diff --git a/docs/index.rst b/docs/index.rst
index ea891237..f2f8dcfd 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -47,6 +47,7 @@ Features and limitations
 #) Encoding is focused on writing to streams. For memory buffers only it could be made more efficient.
 #) The deprecated Protocol Buffers feature called "groups" is not supported.
 #) Fields in the generated structs are ordered by the tag number, instead of the natural ordering in .proto file.
+#) Unknown fields are not preserved when decoding and re-encoding a message.
 
 Getting started
 ===============
diff --git a/docs/reference.rst b/docs/reference.rst
index 2812958f..2fc3d481 100644
--- a/docs/reference.rst
+++ b/docs/reference.rst
@@ -61,7 +61,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.
+:data_size:     Size of a single data entry, in bytes. For PB_LTYPE_BYTES, the size of the byte array inside the containing structure.
 :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.
 
@@ -339,7 +339,7 @@ Read and decode all fields of a structure. Reads until EOF on input stream. ::
 :dest_struct:   Pointer to structure where data will be stored.
 :returns:       True on success, false on IO error, on detectable errors in field description, if a field encoder returns false or if a required field is missing.
 
-In Protocol Buffers binary format, EOF is only allowed between fields. If it happens anywhere else, pb_decode will return *false*.
+In Protocol Buffers binary format, EOF is only allowed between fields. If it happens anywhere else, pb_decode will return *false*. If pb_decode returns false, you cannot trust any of the data in the structure.
 
 In addition to EOF, the pb_decode implementation supports terminating a message with a 0 byte. This is compatible with the official Protocol Buffers because 0 is never a valid field tag.
 
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index a6d38c0b..28268512 100644
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -218,6 +218,9 @@ class Field:
             result += '\n    pb_membersize(%s, %s[0]),' % (self.struct_name, self.name)
             result += ('\n    pb_membersize(%s, %s) / pb_membersize(%s, %s[0]),'
                        % (self.struct_name, self.name, self.struct_name, self.name))
+        elif self.ltype == 'PB_LTYPE_BYTES':
+            result += '\n    pb_membersize(%s, bytes),' % self.ctype
+            result += ' 0,'
         else:
             result += '\n    pb_membersize(%s, %s),' % (self.struct_name, self.name)
             result += ' 0,'
diff --git a/pb_decode.c b/pb_decode.c
index 2cde54c2..9208bdac 100644
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -73,7 +73,7 @@ static bool pb_decode_varint32(pb_istream_t *stream, uint32_t *dest)
 bool pb_decode_varint(pb_istream_t *stream, uint64_t *dest)
 {
     uint8_t byte;
-    int bitpos = 0;
+    uint8_t bitpos = 0;
     *dest = 0;
     
     while (bitpos < 64 && pb_read(stream, &byte, 1))
@@ -460,9 +460,7 @@ bool pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest)
         return false;
     x->size = temp;
     
-    /* Note: data_size includes the size of the x.size field, too.
-     * Calculate actual size starting from offset. */
-    if (x->size > field->data_size - offsetof(pb_bytes_array_t, bytes))
+    if (x->size > field->data_size)
         return false;
     
     return pb_read(stream, x->bytes, x->size);
diff --git a/tests/Makefile b/tests/Makefile
index 243dbd17..115364b6 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,22 +1,32 @@
-CFLAGS=-ansi -Wall -Werror -I .. -g -O0
-DEPS=../pb_decode.c ../pb_decode.h ../pb_encode.c ../pb_encode.h ../pb.h person.h unittests.h
+CFLAGS=-ansi -Wall -Werror -I .. -g -O0 --coverage
+LDFLAGS=--coverage
+DEPS=../pb_decode.h ../pb_encode.h ../pb.h person.h unittests.h
 TESTS=test_decode1 test_encode1 decode_unittests encode_unittests
 
 all: $(TESTS) run_unittests breakpoints
 
 clean:
-	rm -f $(TESTS)
+	rm -f $(TESTS) *.o *.gcda *.gcno
 
-%: %.c $(DEPS)
-	$(CC) $(CFLAGS) -o $@ $< ../pb_decode.c ../pb_encode.c person.c
+%.o: %.c $(DEPS)
+	$(CC) $(CFLAGS) -c -o $@ $<
 
-person.h: person.proto
+test_decode1: test_decode1.o ../pb_decode.o person.o
+test_encode1: test_encode1.o ../pb_encode.o person.o
+decode_unittests: decode_unittests.o ../pb_decode.o person.o
+encode_unittests: encode_unittests.o ../pb_encode.o person.o
+
+person.c person.h: person.proto
 	protoc -I. -I../generator -I/usr/include -operson.pb $<
 	python ../generator/nanopb_generator.py person.pb
 
 breakpoints: ../*.c *.c
 	grep -n 'return false;' $^ | cut -d: -f-2 | xargs -n 1 echo b > $@
 
+coverage:
+	gcov -o .. ../pb_encode.c
+	gcov -o .. ../pb_decode.c
+
 run_unittests: decode_unittests encode_unittests test_encode1 test_decode1
 	./decode_unittests > /dev/null
 	./encode_unittests > /dev/null
diff --git a/tests/decode_unittests.c b/tests/decode_unittests.c
index ac51d491..e733379d 100644
--- a/tests/decode_unittests.c
+++ b/tests/decode_unittests.c
@@ -3,7 +3,7 @@
 #include "pb_decode.h"
 #include "unittests.h"
 
-#define S(x) pb_istream_from_buffer((uint8_t*)x, sizeof(x))
+#define S(x) pb_istream_from_buffer((uint8_t*)x, sizeof(x) - 1)
 
 bool stream_callback(pb_istream_t *stream, uint8_t *buf, size_t count)
 {
@@ -68,17 +68,17 @@ int main()
     {
         pb_istream_t s;
         COMMENT("Test pb_skip_varint");
-        TEST((s = S("\x00""foobar"), pb_skip_varint(&s) && s.bytes_left == 7))
-        TEST((s = S("\xAC\x02""foobar"), pb_skip_varint(&s) && s.bytes_left == 7))
+        TEST((s = S("\x00""foobar"), pb_skip_varint(&s) && s.bytes_left == 6))
+        TEST((s = S("\xAC\x02""foobar"), pb_skip_varint(&s) && s.bytes_left == 6))
         TEST((s = S("\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x01""foobar"),
-              pb_skip_varint(&s) && s.bytes_left == 7))
+              pb_skip_varint(&s) && s.bytes_left == 6))
     }
     
     {
         pb_istream_t s;
         COMMENT("Test pb_skip_string")
-        TEST((s = S("\x00""foobar"), pb_skip_string(&s) && s.bytes_left == 7))
-        TEST((s = S("\x04""testfoobar"), pb_skip_string(&s) && s.bytes_left == 7))
+        TEST((s = S("\x00""foobar"), pb_skip_string(&s) && s.bytes_left == 6))
+        TEST((s = S("\x04""testfoobar"), pb_skip_string(&s) && s.bytes_left == 6))
     }
     
     {
@@ -118,6 +118,53 @@ int main()
         TEST((s = S("\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x01"), pb_dec_svarint(&s, &f, &d) && d == INT64_MIN))
     }
     
+    {
+        pb_istream_t s;
+        pb_field_t f = {1, PB_LTYPE_FIXED, 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))
+    }
+    
+    {
+        pb_istream_t s;
+        pb_field_t f = {1, PB_LTYPE_FIXED, 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))
+    }
+    
+    {
+        pb_istream_t s;
+        struct { size_t size; uint8_t bytes[5]; } d;
+        pb_field_t f = {1, PB_LTYPE_BYTES, 0, 0, 5, 0, 0};
+        
+        COMMENT("Test pb_dec_bytes")
+        TEST((s = S("\x00"), pb_dec_bytes(&s, &f, &d) && d.size == 0))
+        TEST((s = S("\x01\xFF"), pb_dec_bytes(&s, &f, &d) && d.size == 1 && d.bytes[0] == 0xFF))
+        TEST((s = S("\x06xxxxxx"), !pb_dec_bytes(&s, &f, &d)))
+        TEST((s = S("\x05xxxxx"), pb_dec_bytes(&s, &f, &d) && d.size == 5))
+        TEST((s = S("\x05xxxx"), !pb_dec_bytes(&s, &f, &d)))
+    }
+    
+    {
+        pb_istream_t s;
+        pb_field_t f = {1, PB_LTYPE_STRING, 0, 0, 5, 0, 0};
+        char d[5];
+        
+        COMMENT("Test pb_dec_string")
+        TEST((s = S("\x00"), pb_dec_string(&s, &f, &d) && d[0] == '\0'))
+        TEST((s = S("\x04xyzz"), pb_dec_string(&s, &f, &d) && strcmp(d, "xyzz") == 0))
+        TEST((s = S("\x05xyzzy"), !pb_dec_string(&s, &f, &d)))
+    }
+    
     if (status != 0)
         fprintf(stdout, "\n\nSome tests FAILED!\n");