diff options
| -rwxr-xr-x | generator/nanopb_generator.py | 6 | ||||
| -rw-r--r-- | pb.h | 12 | ||||
| -rw-r--r-- | pb_decode.c | 58 | ||||
| -rw-r--r-- | pb_encode.c | 42 | ||||
| -rw-r--r-- | tests/alltypes_pointer/SConscript | 4 | ||||
| -rw-r--r-- | tests/alltypes_pointer/decode_alltypes_pointer.c | 3 | ||||
| -rw-r--r-- | tests/alltypes_pointer/encode_alltypes_pointer.c | 11 |
7 files changed, 59 insertions, 77 deletions
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index 0926db29..c32b26ad 100755 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -246,7 +246,7 @@ class Field: self.ctype = self.struct_name + self.name + 't' self.enc_size = varint_max_size(self.max_size) + self.max_size elif self.allocation == 'POINTER': - self.ctype = 'pb_bytes_ptr_t' + self.ctype = 'pb_bytes_array_t' elif desc.type == FieldD.TYPE_MESSAGE: self.pbtype = 'MESSAGE' self.ctype = self.submsgname = names_from_type_name(desc.type_name) @@ -266,8 +266,8 @@ class Field: if self.pbtype == 'MESSAGE': # Use struct definition, so recursive submessages are possible result += ' struct _%s *%s;' % (self.ctype, self.name) - elif self.rules == 'REPEATED' and self.pbtype == 'STRING': - # String arrays need to be defined as pointers to pointers + elif self.rules == 'REPEATED' and self.pbtype in ['STRING', 'BYTES']: + # String/bytes arrays need to be defined as pointers to pointers result += ' %s **%s;' % (self.ctype, self.name) else: result += ' %s *%s;' % (self.ctype, self.name) @@ -238,21 +238,15 @@ STATIC_ASSERT(sizeof(uint64_t) == 8, UINT64_T_WRONG_SIZE) * It has the number of bytes in the beginning, and after that an array. * Note that actual structs used will have a different length of bytes array. */ +#define PB_BYTES_ARRAY_T(n) struct { size_t size; uint8_t bytes[n]; } +#define PB_BYTES_ARRAY_T_ALLOCSIZE(n) ((size_t)n + offsetof(pb_bytes_array_t, bytes)) + struct _pb_bytes_array_t { size_t size; uint8_t bytes[1]; }; typedef struct _pb_bytes_array_t pb_bytes_array_t; -/* Same, except for pointer-type fields. There is no need to variable struct - * length in this case. - */ -struct _pb_bytes_ptr_t { - size_t size; - uint8_t *bytes; -}; -typedef struct _pb_bytes_ptr_t pb_bytes_ptr_t; - /* This structure is used for giving the callback function. * It is stored in the message structure and filled in by the method that * calls pb_decode. diff --git a/pb_decode.c b/pb_decode.c index 68497406..81febb9b 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -491,13 +491,10 @@ static bool checkreturn allocate_field(pb_istream_t *stream, void *pData, size_t /* Clear a newly allocated item in case it contains a pointer, or is a submessage. */ static void initialize_pointer_field(void *pItem, pb_field_iterator_t *iter) { - if (PB_LTYPE(iter->pos->type) == PB_LTYPE_STRING) + if (PB_LTYPE(iter->pos->type) == PB_LTYPE_STRING || + PB_LTYPE(iter->pos->type) == PB_LTYPE_BYTES) { - *(char**)pItem = NULL; - } - else if (PB_LTYPE(iter->pos->type) == PB_LTYPE_BYTES) - { - memset(pItem, 0, iter->pos->data_size); + *(void**)pItem = NULL; } else if (PB_LTYPE(iter->pos->type) == PB_LTYPE_SUBMESSAGE) { @@ -523,7 +520,8 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_ { case PB_HTYPE_REQUIRED: case PB_HTYPE_OPTIONAL: - if (PB_LTYPE(type) == PB_LTYPE_STRING) + if (PB_LTYPE(type) == PB_LTYPE_STRING || + PB_LTYPE(type) == PB_LTYPE_BYTES) { return func(stream, iter->pos, iter->pData); } @@ -930,10 +928,11 @@ void pb_release(const pb_field_t fields[], void *dest_struct) if (PB_ATYPE(type) == PB_ATYPE_POINTER) { - if (PB_LTYPE(type) == PB_LTYPE_STRING && - PB_HTYPE(type) == PB_HTYPE_REPEATED) + if (PB_HTYPE(type) == PB_HTYPE_REPEATED && + (PB_LTYPE(type) == PB_LTYPE_STRING || + PB_LTYPE(type) == PB_LTYPE_BYTES)) { - /* Release entries in repeated string array */ + /* Release entries in repeated string or bytes array */ void **pItem = *(void***)iter.pData; size_t count = *(size_t*)iter.pSize; while (count--) @@ -942,24 +941,6 @@ void pb_release(const pb_field_t fields[], void *dest_struct) *pItem++ = NULL; } } - else if (PB_LTYPE(type) == PB_LTYPE_BYTES) - { - /* Release entries in repeated bytes array */ - pb_bytes_ptr_t *pItem = *(pb_bytes_ptr_t**)iter.pData; - size_t count = (pItem ? 1 : 0); - - if (PB_HTYPE(type) == PB_HTYPE_REPEATED) - { - count = *(size_t*)iter.pSize; - } - - while (count--) - { - free(pItem->bytes); - pItem->bytes = NULL; - pItem++; - } - } else if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE) { /* Release fields in submessages */ @@ -1109,35 +1090,30 @@ bool checkreturn pb_dec_fixed64(pb_istream_t *stream, const pb_field_t *field, v bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest) { uint32_t size; - size_t alloc_size; + pb_bytes_array_t *bdest; if (!pb_decode_varint32(stream, &size)) return false; - /* Space for the size_t header */ - alloc_size = size + offsetof(pb_bytes_array_t, bytes); - if (PB_ATYPE(field->type) == PB_ATYPE_POINTER) { #ifndef PB_ENABLE_MALLOC PB_RETURN_ERROR(stream, "no malloc support"); #else - pb_bytes_ptr_t *bdest = (pb_bytes_ptr_t*)dest; - if (!allocate_field(stream, &bdest->bytes, alloc_size, 1)) + if (!allocate_field(stream, dest, PB_BYTES_ARRAY_T_ALLOCSIZE(size), 1)) return false; - - bdest->size = size; - return pb_read(stream, bdest->bytes, size); + bdest = *(pb_bytes_array_t**)dest; #endif } else { - pb_bytes_array_t* bdest = (pb_bytes_array_t*)dest; - if (alloc_size > field->data_size) + if (PB_BYTES_ARRAY_T_ALLOCSIZE(size) > field->data_size) PB_RETURN_ERROR(stream, "bytes overflow"); - bdest->size = size; - return pb_read(stream, bdest->bytes, size); + bdest = (pb_bytes_array_t*)dest; } + + bdest->size = size; + return pb_read(stream, bdest->bytes, size); } bool checkreturn pb_dec_string(pb_istream_t *stream, const pb_field_t *field, void *dest) diff --git a/pb_encode.c b/pb_encode.c index c2d0e2c4..59e6f2a7 100644 --- a/pb_encode.c +++ b/pb_encode.c @@ -174,11 +174,12 @@ static bool checkreturn encode_array(pb_ostream_t *stream, const pb_field_t *fie return false; /* Normally the data is stored directly in the array entries, but - * for pointer-type string fields, the array entries are actually - * string pointers. So we have to dereference once more to get to - * the character data. */ + * for pointer-type string and bytes fields, the array entries are + * actually pointers themselves also. So we have to dereference once + * more to get to the actual data. */ if (PB_ATYPE(field->type) == PB_ATYPE_POINTER && - PB_LTYPE(field->type) == PB_LTYPE_STRING) + (PB_LTYPE(field->type) == PB_LTYPE_STRING || + PB_LTYPE(field->type) == PB_LTYPE_BYTES)) { if (!func(stream, field, *(const void* const*)p)) return false; @@ -603,19 +604,21 @@ bool checkreturn pb_enc_fixed32(pb_ostream_t *stream, const pb_field_t *field, c bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src) { - if (PB_ATYPE(field->type) == PB_ATYPE_POINTER) + const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)src; + + if (src == NULL) { - const pb_bytes_ptr_t *bytes = (const pb_bytes_ptr_t*)src; - return pb_encode_string(stream, bytes->bytes, bytes->size); + /* Threat null pointer as an empty bytes field */ + return pb_encode_string(stream, NULL, 0); } - else + + if (PB_ATYPE(field->type) == PB_ATYPE_STATIC && + PB_BYTES_ARRAY_T_ALLOCSIZE(bytes->size) > field->data_size) { - const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)src; - if (bytes->size + offsetof(pb_bytes_array_t, bytes) > field->data_size) - PB_RETURN_ERROR(stream, "bytes size exceeded"); - - return pb_encode_string(stream, bytes->bytes, bytes->size); + PB_RETURN_ERROR(stream, "bytes size exceeded"); } + + return pb_encode_string(stream, bytes->bytes, bytes->size); } bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *field, const void *src) @@ -628,10 +631,17 @@ bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *field, co if (PB_ATYPE(field->type) == PB_ATYPE_POINTER) max_size = (size_t)-1; - while (size < max_size && *p != '\0') + if (src == NULL) { - size++; - p++; + size = 0; /* Threat null pointer as an empty string */ + } + else + { + while (size < max_size && *p != '\0') + { + size++; + p++; + } } return pb_encode_string(stream, (const uint8_t*)src, size); diff --git a/tests/alltypes_pointer/SConscript b/tests/alltypes_pointer/SConscript index 45985ff5..97e4267a 100644 --- a/tests/alltypes_pointer/SConscript +++ b/tests/alltypes_pointer/SConscript @@ -30,9 +30,11 @@ refdec = "$BUILD/alltypes/decode_alltypes$PROGSUFFIX" # Encode and compare results env.RunTest(enc) +env.Compare(["encode_alltypes_pointer.output", "$BUILD/alltypes/encode_alltypes.output"]) + +# Decode env.RunTest("decode_alltypes.output", [dec, "encode_alltypes_pointer.output"]) env.RunTest("decode_alltypes_ref.output", [refdec, "encode_alltypes_pointer.output"]) -env.Compare(["encode_alltypes_pointer.output", "$BUILD/alltypes/encode_alltypes.output"]) # Do the same thing with the optional fields present env.RunTest("optionals.output", enc, ARGS = ['1']) diff --git a/tests/alltypes_pointer/decode_alltypes_pointer.c b/tests/alltypes_pointer/decode_alltypes_pointer.c index 29495ac4..47d72685 100644 --- a/tests/alltypes_pointer/decode_alltypes_pointer.c +++ b/tests/alltypes_pointer/decode_alltypes_pointer.c @@ -48,8 +48,7 @@ bool check_alltypes(pb_istream_t *stream, int mode) TEST(alltypes.req_string && strcmp(alltypes.req_string, "1014") == 0); TEST(alltypes.req_bytes && alltypes.req_bytes->size == 4); - TEST(alltypes.req_bytes && alltypes.req_bytes->bytes - && memcmp(alltypes.req_bytes->bytes, "1015", 4) == 0); + TEST(alltypes.req_bytes && memcmp(&alltypes.req_bytes->bytes, "1015", 4) == 0); TEST(alltypes.req_submsg && alltypes.req_submsg->substuff1 && strcmp(alltypes.req_submsg->substuff1, "1016") == 0); TEST(alltypes.req_submsg && alltypes.req_submsg->substuff2 diff --git a/tests/alltypes_pointer/encode_alltypes_pointer.c b/tests/alltypes_pointer/encode_alltypes_pointer.c index 6484ced8..c128569b 100644 --- a/tests/alltypes_pointer/encode_alltypes_pointer.c +++ b/tests/alltypes_pointer/encode_alltypes_pointer.c @@ -27,7 +27,7 @@ int main(int argc, char **argv) int64_t req_sfixed64 = -1012; double req_double = 1013.0; char* req_string = "1014"; - pb_bytes_ptr_t req_bytes = {4, (uint8_t*)"1015"}; + PB_BYTES_ARRAY_T(4) req_bytes = {4, {'1', '0', '1', '5'}}; static int32_t req_substuff = 1016; SubMessage req_submsg = {"1016", &req_substuff}; MyEnum req_enum = MyEnum_Truth; @@ -50,7 +50,8 @@ int main(int argc, char **argv) int64_t rep_sfixed64[5] = {0, 0, 0, 0, -2012}; double rep_double[5] = {0, 0, 0, 0, 2013.0f}; char* rep_string[5] = {"", "", "", "", "2014"}; - pb_bytes_ptr_t rep_bytes[5] = {{0,0}, {0,0}, {0,0}, {0,0}, {4, (uint8_t*)"2015"}}; + static PB_BYTES_ARRAY_T(4) rep_bytes_4 = {4, {'2', '0', '1', '5'}}; + pb_bytes_array_t *rep_bytes[5]= {NULL, NULL, NULL, NULL, (pb_bytes_array_t*)&rep_bytes_4}; static int32_t rep_sub2zero = 0; static int32_t rep_substuff2 = 2016; static uint32_t rep_substuff3 = 2016; @@ -77,7 +78,7 @@ int main(int argc, char **argv) int64_t opt_sfixed64 = 3052; double opt_double = 3053.0; char* opt_string = "3054"; - pb_bytes_ptr_t opt_bytes = {4, (uint8_t*)"3055"}; + PB_BYTES_ARRAY_T(4) opt_bytes = {4, {'3', '0', '5', '5'}}; static int32_t opt_substuff = 3056; SubMessage opt_submsg = {"3056", &opt_substuff}; MyEnum opt_enum = MyEnum_Truth; @@ -117,7 +118,7 @@ int main(int argc, char **argv) alltypes.req_sfixed64 = &req_sfixed64; alltypes.req_double = &req_double; alltypes.req_string = req_string; - alltypes.req_bytes = &req_bytes; + alltypes.req_bytes = (pb_bytes_array_t*)&req_bytes; alltypes.req_submsg = &req_submsg; alltypes.req_enum = &req_enum; alltypes.req_emptymsg = &req_emptymsg; @@ -159,7 +160,7 @@ int main(int argc, char **argv) alltypes.opt_sfixed64 = &opt_sfixed64; alltypes.opt_double = &opt_double; alltypes.opt_string = opt_string; - alltypes.opt_bytes = &opt_bytes; + alltypes.opt_bytes = (pb_bytes_array_t*)&opt_bytes; alltypes.opt_submsg = &opt_submsg; alltypes.opt_enum = &opt_enum; alltypes.opt_emptymsg = &opt_emptymsg; |