rik/nanopb
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Get rid of pb_bytes_ptr_t, just allocate pb_bytes_array_t dynamically.

Browse Source 
This makes the internal logic much simpler, and also keeps the datatypes
more similar between STATIC/POINTER cases. It will still be a bit cumbersome
to use because of variable length array member. Macros PB_BYTES_ARRAY_T(n) and
PB_BYTES_ARRAY_T_ALLOCSIZE(n) have been added to make life a bit easier.

This has the drawback that it is no longer as easy to use externally allocated
byte array as input for bytes field in pointer mode. However, this is still
easy to do using callbacks, so it shouldn't be a large issue.
This commit is contained in:
Petteri Aimonen
2014-03-15 08:45:58 +02:00
parent 9c196b89ba
commit 9be2cfe968
7 changed files with 59 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
generator/nanopb_generator.py
View File

@@ -246,7 +246,7 @@ class Field:
self.ctype = self.struct_name + self.name + 't' self.ctype = self.struct_name + self.name + 't'
self.enc_size = varint_max_size(self.max_size) + self.max_size self.enc_size = varint_max_size(self.max_size) + self.max_size
elif self.allocation == 'POINTER': elif self.allocation == 'POINTER':
self.ctype = 'pb_bytes_ptr_t' self.ctype = 'pb_bytes_array_t'
elif desc.type == FieldD.TYPE_MESSAGE: elif desc.type == FieldD.TYPE_MESSAGE:
self.pbtype = 'MESSAGE' self.pbtype = 'MESSAGE'
self.ctype = self.submsgname = names_from_type_name(desc.type_name) self.ctype = self.submsgname = names_from_type_name(desc.type_name)
@@ -266,8 +266,8 @@ class Field:
if self.pbtype == 'MESSAGE': if self.pbtype == 'MESSAGE':
# Use struct definition, so recursive submessages are possible # Use struct definition, so recursive submessages are possible
result += ' struct _%s *%s;' % (self.ctype, self.name) result += ' struct _%s *%s;' % (self.ctype, self.name)
elif self.rules == 'REPEATED' and self.pbtype == 'STRING': elif self.rules == 'REPEATED' and self.pbtype in ['STRING', 'BYTES']:
# String arrays need to be defined as pointers to pointers # String/bytes arrays need to be defined as pointers to pointers
result += ' %s **%s;' % (self.ctype, self.name) result += ' %s **%s;' % (self.ctype, self.name)
else: else:
result += ' %s *%s;' % (self.ctype, self.name) result += ' %s *%s;' % (self.ctype, self.name)

12
pb.h
View File

@@ -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. * 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. * 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 { struct _pb_bytes_array_t {
size_t size; size_t size;
uint8_t bytes[1]; uint8_t bytes[1];
}; };
typedef struct _pb_bytes_array_t pb_bytes_array_t; 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. /* This structure is used for giving the callback function.
* It is stored in the message structure and filled in by the method that * It is stored in the message structure and filled in by the method that
* calls pb_decode. * calls pb_decode.

58
pb_decode.c
View File

@@ -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. */ /* 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) 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; *(void**)pItem = NULL;
}
else if (PB_LTYPE(iter->pos->type) == PB_LTYPE_BYTES)
{
memset(pItem, 0, iter->pos->data_size);
} }
else if (PB_LTYPE(iter->pos->type) == PB_LTYPE_SUBMESSAGE) 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_REQUIRED:
case PB_HTYPE_OPTIONAL: 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); 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_ATYPE(type) == PB_ATYPE_POINTER)
{ {
if (PB_LTYPE(type) == PB_LTYPE_STRING && if (PB_HTYPE(type) == PB_HTYPE_REPEATED &&
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; void **pItem = *(void***)iter.pData;
size_t count = *(size_t*)iter.pSize; size_t count = *(size_t*)iter.pSize;
while (count--) while (count--)
@@ -942,24 +941,6 @@ void pb_release(const pb_field_t fields[], void *dest_struct)
*pItem++ = NULL; *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) else if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE)
{ {
/* Release fields in submessages */ /* 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) bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest)
{ {
uint32_t size; uint32_t size;
size_t alloc_size; pb_bytes_array_t *bdest;
if (!pb_decode_varint32(stream, &size)) if (!pb_decode_varint32(stream, &size))
return false; 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) if (PB_ATYPE(field->type) == PB_ATYPE_POINTER)
{ {
#ifndef PB_ENABLE_MALLOC #ifndef PB_ENABLE_MALLOC
PB_RETURN_ERROR(stream, "no malloc support"); PB_RETURN_ERROR(stream, "no malloc support");
#else #else
pb_bytes_ptr_t *bdest = (pb_bytes_ptr_t*)dest; if (!allocate_field(stream, dest, PB_BYTES_ARRAY_T_ALLOCSIZE(size), 1))
if (!allocate_field(stream, &bdest->bytes, alloc_size, 1))
return false; return false;
bdest = *(pb_bytes_array_t**)dest;
bdest->size = size;
return pb_read(stream, bdest->bytes, size);
#endif #endif
} }
else else
{ {
pb_bytes_array_t* bdest = (pb_bytes_array_t*)dest; if (PB_BYTES_ARRAY_T_ALLOCSIZE(size) > field->data_size)
if (alloc_size > field->data_size)
PB_RETURN_ERROR(stream, "bytes overflow"); PB_RETURN_ERROR(stream, "bytes overflow");
bdest->size = size; bdest = (pb_bytes_array_t*)dest;
return pb_read(stream, bdest->bytes, size);
} }
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) bool checkreturn pb_dec_string(pb_istream_t *stream, const pb_field_t *field, void *dest)

46
pb_encode.c
View File

@@ -174,11 +174,12 @@ static bool checkreturn encode_array(pb_ostream_t *stream, const pb_field_t *fie
return false; return false;
/* Normally the data is stored directly in the array entries, but /* Normally the data is stored directly in the array entries, but
* for pointer-type string fields, the array entries are actually * for pointer-type string and bytes fields, the array entries are
* string pointers. So we have to dereference once more to get to * actually pointers themselves also. So we have to dereference once
* the character data. */ * more to get to the actual data. */
if (PB_ATYPE(field->type) == PB_ATYPE_POINTER && 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)) if (!func(stream, field, *(const void* const*)p))
return false; 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) 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;
{
const pb_bytes_ptr_t *bytes = (const pb_bytes_ptr_t*)src;
return pb_encode_string(stream, bytes->bytes, bytes->size);
}
else
{
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); if (src == NULL)
{
/* Threat null pointer as an empty bytes field */
return pb_encode_string(stream, NULL, 0);
} }
if (PB_ATYPE(field->type) == PB_ATYPE_STATIC &&
PB_BYTES_ARRAY_T_ALLOCSIZE(bytes->size) > field->data_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) 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) if (PB_ATYPE(field->type) == PB_ATYPE_POINTER)
max_size = (size_t)-1; max_size = (size_t)-1;
while (size < max_size && *p != '\0') if (src == NULL)
{ {
size++; size = 0; /* Threat null pointer as an empty string */
p++; }
else
{
while (size < max_size && *p != '\0')
{
size++;
p++;
}
} }
return pb_encode_string(stream, (const uint8_t*)src, size); return pb_encode_string(stream, (const uint8_t*)src, size);

4
tests/alltypes_pointer/SConscript
View File

@@ -30,9 +30,11 @@ refdec = "$BUILD/alltypes/decode_alltypes$PROGSUFFIX"
# Encode and compare results # Encode and compare results
env.RunTest(enc) 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.output", [dec, "encode_alltypes_pointer.output"])
env.RunTest("decode_alltypes_ref.output", [refdec, "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 # Do the same thing with the optional fields present
env.RunTest("optionals.output", enc, ARGS = ['1']) env.RunTest("optionals.output", enc, ARGS = ['1'])

3
tests/alltypes_pointer/decode_alltypes_pointer.c
View File

@@ -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_string && strcmp(alltypes.req_string, "1014") == 0);
TEST(alltypes.req_bytes && alltypes.req_bytes->size == 4); TEST(alltypes.req_bytes && alltypes.req_bytes->size == 4);
TEST(alltypes.req_bytes && alltypes.req_bytes->bytes TEST(alltypes.req_bytes && memcmp(&alltypes.req_bytes->bytes, "1015", 4) == 0);
&& memcmp(alltypes.req_bytes->bytes, "1015", 4) == 0);
TEST(alltypes.req_submsg && alltypes.req_submsg->substuff1 TEST(alltypes.req_submsg && alltypes.req_submsg->substuff1
&& strcmp(alltypes.req_submsg->substuff1, "1016") == 0); && strcmp(alltypes.req_submsg->substuff1, "1016") == 0);
TEST(alltypes.req_submsg && alltypes.req_submsg->substuff2 TEST(alltypes.req_submsg && alltypes.req_submsg->substuff2

11
tests/alltypes_pointer/encode_alltypes_pointer.c
View File

@@ -27,7 +27,7 @@ int main(int argc, char **argv)
int64_t req_sfixed64 = -1012; int64_t req_sfixed64 = -1012;
double req_double = 1013.0; double req_double = 1013.0;
char* req_string = "1014"; 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; static int32_t req_substuff = 1016;
SubMessage req_submsg = {"1016", &req_substuff}; SubMessage req_submsg = {"1016", &req_substuff};
MyEnum req_enum = MyEnum_Truth; 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}; int64_t rep_sfixed64[5] = {0, 0, 0, 0, -2012};
double rep_double[5] = {0, 0, 0, 0, 2013.0f}; double rep_double[5] = {0, 0, 0, 0, 2013.0f};
char* rep_string[5] = {"", "", "", "", "2014"}; 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_sub2zero = 0;
static int32_t rep_substuff2 = 2016; static int32_t rep_substuff2 = 2016;
static uint32_t rep_substuff3 = 2016; static uint32_t rep_substuff3 = 2016;
@@ -77,7 +78,7 @@ int main(int argc, char **argv)
int64_t opt_sfixed64 = 3052; int64_t opt_sfixed64 = 3052;
double opt_double = 3053.0; double opt_double = 3053.0;
char* opt_string = "3054"; 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; static int32_t opt_substuff = 3056;
SubMessage opt_submsg = {"3056", &opt_substuff}; SubMessage opt_submsg = {"3056", &opt_substuff};
MyEnum opt_enum = MyEnum_Truth; MyEnum opt_enum = MyEnum_Truth;
@@ -117,7 +118,7 @@ int main(int argc, char **argv)
alltypes.req_sfixed64 = &req_sfixed64; alltypes.req_sfixed64 = &req_sfixed64;
alltypes.req_double = &req_double; alltypes.req_double = &req_double;
alltypes.req_string = req_string; 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_submsg = &req_submsg;
alltypes.req_enum = &req_enum; alltypes.req_enum = &req_enum;
alltypes.req_emptymsg = &req_emptymsg; alltypes.req_emptymsg = &req_emptymsg;
@@ -159,7 +160,7 @@ int main(int argc, char **argv)
alltypes.opt_sfixed64 = &opt_sfixed64; alltypes.opt_sfixed64 = &opt_sfixed64;
alltypes.opt_double = &opt_double; alltypes.opt_double = &opt_double;
alltypes.opt_string = opt_string; 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_submsg = &opt_submsg;
alltypes.opt_enum = &opt_enum; alltypes.opt_enum = &opt_enum;
alltypes.opt_emptymsg = &opt_emptymsg; alltypes.opt_emptymsg = &opt_emptymsg;