From 1dd9f1900fca0c137324c05a9421f1ba180b2470 Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Mon, 18 Aug 2014 20:09:52 +0300 Subject: [PATCH] Change the _count fields to use pb_size_t datatype. Update issue 82 Status: FixedInGit --- docs/migration.rst | 15 ++++++++++ generator/nanopb_generator.py | 9 ++---- pb.h | 7 +++-- pb_decode.c | 40 ++++++++++++++++++------- pb_encode.c | 4 +-- tests/backwards_compatibility/alltypes_legacy.h | 40 ++++++++++++------------- tests/decode_unittests/decode_unittests.c | 4 +-- tests/encode_unittests/encode_unittests.c | 2 +- 8 files changed, 77 insertions(+), 44 deletions(-) diff --git a/docs/migration.rst b/docs/migration.rst index 9d41f5b1..800bc1f9 100644 --- a/docs/migration.rst +++ b/docs/migration.rst @@ -31,6 +31,21 @@ functionality is not needed. **Error indications:** Linker error: undefined reference to *pb_field_iter_begin*, *pb_field_iter_next* or similar. +Change data type of field counts to pb_size_t +--------------------------------------------- +**Rationale:** Often nanopb is used with small arrays, such as 255 items or +less. Using a full *size_t* field to store the array count wastes memory if +there are many arrays. There already exists parameters *PB_FIELD_16BIT* and +*PB_FIELD_32BIT* which tell nanopb what is the maximum size of arrays in use. + +**Changes:** Generator will now use *pb_size_t* for the array *_count* fields. +The size of the type will be controlled by the *PB_FIELD_16BIT* and +*PB_FIELD_32BIT* compilation time options. + +**Required actions:** Regenerate all *.pb.h* files. In some cases casts to the +*pb_size_t* type may need to be added in the user code when accessing the +*_count* fields. + Nanopb-0.2.9 (2014-08-09) ========================= diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index 1d12ae3c..88e9798d 100755 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -261,7 +261,7 @@ class Field: result = '' if self.allocation == 'POINTER': if self.rules == 'REPEATED': - result += ' size_t ' + self.name + '_count;\n' + result += ' pb_size_t ' + self.name + '_count;\n' if self.pbtype == 'MESSAGE': # Use struct definition, so recursive submessages are possible @@ -277,17 +277,14 @@ class Field: if self.rules == 'OPTIONAL' and self.allocation == 'STATIC': result += ' bool has_' + self.name + ';\n' elif self.rules == 'REPEATED' and self.allocation == 'STATIC': - result += ' size_t ' + self.name + '_count;\n' + result += ' pb_size_t ' + self.name + '_count;\n' result += ' %s %s%s;' % (self.ctype, self.name, self.array_decl) return result def types(self): '''Return definitions for any special types this field might need.''' if self.pbtype == 'BYTES' and self.allocation == 'STATIC': - result = 'typedef struct {\n' - result += ' size_t size;\n' - result += ' uint8_t bytes[%d];\n' % self.max_size - result += '} %s;\n' % self.ctype + result = 'typedef PB_BYTES_ARRAY_T(%d) %s;\n' % (self.max_size, self.ctype) else: result = None return result diff --git a/pb.h b/pb.h index 50a07c54..c9ef48b9 100644 --- a/pb.h +++ b/pb.h @@ -187,12 +187,15 @@ typedef uint8_t pb_type_t; * and array counts. */ #if defined(PB_FIELD_32BIT) +#define PB_SIZE_MAX ((uint32_t)-1) typedef uint32_t pb_size_t; typedef int32_t pb_ssize_t; #elif defined(PB_FIELD_16BIT) +#define PB_SIZE_MAX ((uint16_t)-1) typedef uint16_t pb_size_t; typedef int16_t pb_ssize_t; #else +#define PB_SIZE_MAX ((uint8_t)-1) typedef uint8_t pb_size_t; typedef int8_t pb_ssize_t; #endif @@ -241,11 +244,11 @@ 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(n) struct { pb_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; + pb_size_t size; uint8_t bytes[1]; }; typedef struct _pb_bytes_array_t pb_bytes_array_t; diff --git a/pb_decode.c b/pb_decode.c index 3e817cc8..63ec0de9 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -352,7 +352,7 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t { /* Packed array */ bool status = true; - size_t *size = (size_t*)iter->pSize; + pb_size_t *size = (pb_size_t*)iter->pSize; pb_istream_t substream; if (!pb_make_string_substream(stream, &substream)) return false; @@ -377,7 +377,7 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t else { /* Repeated field */ - size_t *size = (size_t*)iter->pSize; + pb_size_t *size = (pb_size_t*)iter->pSize; void *pItem = (uint8_t*)iter->pData + iter->pos->data_size * (*size); if (*size >= iter->pos->array_size) PB_RETURN_ERROR(stream, "array overflow"); @@ -478,7 +478,7 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_ { /* Packed array, multiple items come in at once. */ bool status = true; - size_t *size = (size_t*)iter->pSize; + pb_size_t *size = (pb_size_t*)iter->pSize; size_t allocated_size = *size; void *pItem; pb_istream_t substream; @@ -488,7 +488,7 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_ while (substream.bytes_left) { - if (*size + 1 > allocated_size) + if ((size_t)*size + 1 > allocated_size) { /* Allocate more storage. This tries to guess the * number of remaining entries. Round the division @@ -510,6 +510,16 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_ status = false; break; } + + if (*size == PB_SIZE_MAX) + { +#ifndef PB_NO_ERRMSG + stream->errmsg = "too many array entries"; +#endif + status = false; + break; + } + (*size)++; } pb_close_string_substream(stream, &substream); @@ -519,9 +529,12 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_ else { /* Normal repeated field, i.e. only one item at a time. */ - size_t *size = (size_t*)iter->pSize; + pb_size_t *size = (pb_size_t*)iter->pSize; void *pItem; + if (*size == PB_SIZE_MAX) + PB_RETURN_ERROR(stream, "too many array entries"); + (*size)++; if (!allocate_field(stream, iter->pData, iter->pos->data_size, *size)) return false; @@ -688,7 +701,7 @@ static void pb_message_set_to_defaults(const pb_field_t fields[], void *dest_str else if (PB_HTYPE(type) == PB_HTYPE_REPEATED) { /* Set array count to 0, no need to initialize contents. */ - *(size_t*)iter.pSize = 0; + *(pb_size_t*)iter.pSize = 0; continue; } @@ -716,7 +729,7 @@ static void pb_message_set_to_defaults(const pb_field_t fields[], void *dest_str /* Initialize array count to 0. */ if (PB_HTYPE(type) == PB_HTYPE_REPEATED) { - *(size_t*)iter.pSize = 0; + *(pb_size_t*)iter.pSize = 0; } } else if (PB_ATYPE(type) == PB_ATYPE_CALLBACK) @@ -876,7 +889,7 @@ void pb_release(const pb_field_t fields[], void *dest_struct) { /* Release entries in repeated string or bytes array */ void **pItem = *(void***)iter.pData; - size_t count = *(size_t*)iter.pSize; + pb_size_t count = *(pb_size_t*)iter.pSize; while (count--) { pb_free(*pItem); @@ -887,11 +900,11 @@ void pb_release(const pb_field_t fields[], void *dest_struct) { /* Release fields in submessages */ void *pItem = *(void**)iter.pData; - size_t count = (pItem ? 1 : 0); + pb_size_t count = (pItem ? 1 : 0); if (PB_HTYPE(type) == PB_HTYPE_REPEATED) { - count = *(size_t*)iter.pSize; + count = *(pb_size_t*)iter.pSize; } while (count--) @@ -1054,7 +1067,12 @@ static bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *fie bdest = (pb_bytes_array_t*)dest; } - bdest->size = size; + if (size > PB_SIZE_MAX) + { + PB_RETURN_ERROR(stream, "bytes overflow"); + } + + bdest->size = (pb_size_t)size; return pb_read(stream, bdest->bytes, size); } diff --git a/pb_encode.c b/pb_encode.c index 6041c641..3dce1c10 100644 --- a/pb_encode.c +++ b/pb_encode.c @@ -246,7 +246,7 @@ static bool checkreturn encode_basic_field(pb_ostream_t *stream, break; case PB_HTYPE_REPEATED: - if (!encode_array(stream, field, pData, *(const size_t*)pSize, func)) + if (!encode_array(stream, field, pData, *(const pb_size_t*)pSize, func)) return false; break; @@ -630,7 +630,6 @@ static bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *fie static bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *field, const void *src) { - /* strnlen() is not always available, so just use a loop */ size_t size = 0; size_t max_size = field->data_size; const char *p = (const char*)src; @@ -644,6 +643,7 @@ static bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *fi } else { + /* strnlen() is not always available, so just use a loop */ while (size < max_size && *p != '\0') { size++; diff --git a/tests/backwards_compatibility/alltypes_legacy.h b/tests/backwards_compatibility/alltypes_legacy.h index 037b3478..abdd97a7 100644 --- a/tests/backwards_compatibility/alltypes_legacy.h +++ b/tests/backwards_compatibility/alltypes_legacy.h @@ -29,17 +29,17 @@ typedef struct _SubMessage { } SubMessage; typedef struct { - size_t size; + pb_size_t size; uint8_t bytes[16]; } AllTypes_req_bytes_t; typedef struct { - size_t size; + pb_size_t size; uint8_t bytes[16]; } AllTypes_rep_bytes_t; typedef struct { - size_t size; + pb_size_t size; uint8_t bytes[16]; } AllTypes_opt_bytes_t; @@ -61,39 +61,39 @@ typedef struct _AllTypes { AllTypes_req_bytes_t req_bytes; SubMessage req_submsg; MyEnum req_enum; - size_t rep_int32_count; + pb_size_t rep_int32_count; int32_t rep_int32[5]; - size_t rep_int64_count; + pb_size_t rep_int64_count; int64_t rep_int64[5]; - size_t rep_uint32_count; + pb_size_t rep_uint32_count; uint32_t rep_uint32[5]; - size_t rep_uint64_count; + pb_size_t rep_uint64_count; uint64_t rep_uint64[5]; - size_t rep_sint32_count; + pb_size_t rep_sint32_count; int32_t rep_sint32[5]; - size_t rep_sint64_count; + pb_size_t rep_sint64_count; int64_t rep_sint64[5]; - size_t rep_bool_count; + pb_size_t rep_bool_count; bool rep_bool[5]; - size_t rep_fixed32_count; + pb_size_t rep_fixed32_count; uint32_t rep_fixed32[5]; - size_t rep_sfixed32_count; + pb_size_t rep_sfixed32_count; int32_t rep_sfixed32[5]; - size_t rep_float_count; + pb_size_t rep_float_count; float rep_float[5]; - size_t rep_fixed64_count; + pb_size_t rep_fixed64_count; uint64_t rep_fixed64[5]; - size_t rep_sfixed64_count; + pb_size_t rep_sfixed64_count; int64_t rep_sfixed64[5]; - size_t rep_double_count; + pb_size_t rep_double_count; double rep_double[5]; - size_t rep_string_count; + pb_size_t rep_string_count; char rep_string[5][16]; - size_t rep_bytes_count; + pb_size_t rep_bytes_count; AllTypes_rep_bytes_t rep_bytes[5]; - size_t rep_submsg_count; + pb_size_t rep_submsg_count; SubMessage rep_submsg[5]; - size_t rep_enum_count; + pb_size_t rep_enum_count; MyEnum rep_enum[5]; bool has_opt_int32; int32_t opt_int32; diff --git a/tests/decode_unittests/decode_unittests.c b/tests/decode_unittests/decode_unittests.c index 59c4a074..97212aff 100644 --- a/tests/decode_unittests/decode_unittests.c +++ b/tests/decode_unittests/decode_unittests.c @@ -170,7 +170,7 @@ int main() { pb_istream_t s; - struct { size_t size; uint8_t bytes[5]; } d; + struct { pb_size_t size; uint8_t bytes[5]; } d; pb_field_t f = {1, PB_LTYPE_BYTES, 0, 0, sizeof(d), 0, 0}; COMMENT("Test pb_dec_bytes") @@ -251,7 +251,7 @@ int main() { pb_istream_t s; CallbackArray dest; - struct { size_t size; uint8_t bytes[10]; } ref; + struct { pb_size_t size; uint8_t bytes[10]; } ref; dest.data.funcs.decode = &callback_check; dest.data.arg = &ref; diff --git a/tests/encode_unittests/encode_unittests.c b/tests/encode_unittests/encode_unittests.c index a5f868c9..78fbb8b1 100644 --- a/tests/encode_unittests/encode_unittests.c +++ b/tests/encode_unittests/encode_unittests.c @@ -170,7 +170,7 @@ int main() { uint8_t buffer[30]; pb_ostream_t s; - struct { size_t size; uint8_t bytes[5]; } value = {5, {'x', 'y', 'z', 'z', 'y'}}; + struct { pb_size_t size; uint8_t bytes[5]; } value = {5, {'x', 'y', 'z', 'z', 'y'}}; COMMENT("Test pb_enc_bytes") TEST(WRITES(pb_enc_bytes(&s, &BytesMessage_fields[0], &value), "\x05xyzzy")) -- 2.16.6