diff --git a/docs/concepts.rst b/docs/concepts.rst index e162b88..f2b3bb2 100644 --- a/docs/concepts.rst +++ b/docs/concepts.rst @@ -250,14 +250,34 @@ generates these declarations and definitions for the structure *Person_PhoneNumb pb_membersize(Person_PhoneNumber, number), 0, 0}, {2, PB_HTYPE_OPTIONAL | PB_LTYPE_VARINT, - pb_delta_end(Person_PhoneNumber, type, number), - pb_delta(Person_PhoneNumber, has_type, type), + pb_delta_end(Person_PhoneNumber, type, number), 0, pb_membersize(Person_PhoneNumber, type), 0, &Person_PhoneNumber_type_default}, } }; + #define Person_PhoneNumber_has(STRUCT, FIELD) PB_HAS_FIELD(STRUCT, Person_PhoneNumber, FIELD) + #define Person_PhoneNumber_set(STRUCT, FIELD) PB_SET_FIELD(STRUCT, Person_PhoneNumber, FIELD) + #define Person_PhoneNumber_clear(STRUCT, FIELD) PB_CLEAR_FIELD(STRUCT, Person_PhoneNumber, FIELD) + #define Person_PhoneNumber_number_index 0 + #define Person_PhoneNumber_number_tag 1 + #define Person_PhoneNumber_type_index 1 + #define Person_PhoneNumber_type_tag 2 + +Optional Fields +=============== + +The *has_fields* member of each generated structure is an array where +each bit indicates the presence of the corresponding (optional) field. +The generated header file provides helper macros to read and update +that array; in the previous example, they are +*Person_PhoneNumber_has*, *Person_PhoneNumber_set* and +*Person_PhoneNumber_clear*. + +For convenience, *pb_encode* only checks these bits for optional +fields. *pb_decode* sets the corresponding bit for every field it +decodes, whether the field is optional or not. Return values and error handling ================================ diff --git a/docs/reference.rst b/docs/reference.rst index 45b190f..6fd2b3c 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -34,8 +34,8 @@ The high-order nibble defines whether the field is required, optional, repeated HTYPE identifier Value Field handling ==================== ===== ================================================ PB_HTYPE_REQUIRED 0x00 Verify that field exists in decoded message. -PB_HTYPE_OPTIONAL 0x10 Use separate *has_* boolean to specify - whether the field is present. +PB_HTYPE_OPTIONAL 0x10 Use the structure's *has_fields* bit array to + specify whether the field is present. PB_HTYPE_ARRAY 0x20 A repeated field with preallocated array. Separate *_count* for number of items. PB_HTYPE_CALLBACK 0x30 A field with dynamic storage size, data is @@ -61,7 +61,7 @@ Describes a single structure field with memory position in relation to others. T :tag: Tag number of the field or 0 to terminate a list of fields. :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. +:size_offset: Offset of *size_t* count for arrays, relative to field data. :data_size: Size of a single data entry, in bytes. For PB_LTYPE_BYTES, the size of the byte array inside the containing structure. For PB_HTYPE_CALLBACK, size of the C data type if known. :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. @@ -366,9 +366,7 @@ In Protocol Buffers binary format, EOF is only allowed between fields. If it hap 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. -For optional fields, this function applies the default value and sets *has_* to false if the field is not present. - -Because of memory concerns, the detection of missing required fields is not perfect if the structure contains more than 32 fields. +For optional fields, this function applies the default value and clears the corresponding bit in *has_fields* if the field is not present. .. sidebar:: Field decoders diff --git a/example/client.c b/example/client.c index 0a9a38a..b0c1c90 100644 --- a/example/client.c +++ b/example/client.c @@ -45,11 +45,11 @@ bool listdir(int fd, char *path) if (path == NULL) { - request.has_path = false; + ListFilesRequest_clear(request, path); } else { - request.has_path = true; + ListFilesRequest_set(request, path); if (strlen(path) + 1 > sizeof(request.path)) { fprintf(stderr, "Too long path.\n"); diff --git a/example/server.c b/example/server.c index 6b36d6d..20d1ee7 100644 --- a/example/server.c +++ b/example/server.c @@ -67,13 +67,13 @@ void handle_connection(int connfd) { perror("opendir"); - response.has_path_error = true; + ListFilesResponse_set(response, path_error); response.path_error = true; response.file.funcs.encode = NULL; } else { - response.has_path_error = false; + ListFilesResponse_clear(response, path_error); response.file.funcs.encode = &listdir_callback; response.file.arg = directory; } diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index 6c8356f..9e21672 100644 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -138,9 +138,7 @@ class Field: return cmp(self.tag, other.tag) def __str__(self): - if self.htype == 'PB_HTYPE_OPTIONAL': - result = ' bool has_' + self.name + ';\n' - elif self.htype == 'PB_HTYPE_ARRAY': + if self.htype == 'PB_HTYPE_ARRAY': result = ' size_t ' + self.name + '_count;\n' else: result = '' @@ -207,9 +205,7 @@ class Field: else: result += ' pb_delta_end(%s, %s, %s),' % (self.struct_name, self.name, prev_field_name) - if self.htype == 'PB_HTYPE_OPTIONAL': - result += '\n pb_delta(%s, has_%s, %s),' % (self.struct_name, self.name, self.name) - elif self.htype == 'PB_HTYPE_ARRAY': + if self.htype == 'PB_HTYPE_ARRAY': result += '\n pb_delta(%s, %s_count, %s),' % (self.struct_name, self.name, self.name) else: result += ' 0,' @@ -248,6 +244,7 @@ class Message: def __str__(self): result = 'typedef struct {\n' + result += ' uint8_t has_fields[%d];\n' % ((len(self.fields) + 7) / 8) result += '\n'.join([str(f) for f in self.ordered_fields]) result += '\n} %s;' % self.name return result @@ -288,6 +285,18 @@ class Message: result += ' }\n};' return result + def field_numbers(self): + result = '#define %s_has(STRUCT, FIELD) PB_HAS_FIELD(STRUCT, %s, FIELD)\n' % (self.name, self.name) + result += '#define %s_set(STRUCT, FIELD) PB_SET_FIELD(STRUCT, %s, FIELD)\n' % (self.name, self.name) + result += '#define %s_clear(STRUCT, FIELD) PB_CLEAR_FIELD(STRUCT, %s, FIELD)\n' % (self.name, self.name) + + i = 0 + for field in self.ordered_fields: + result += '#define %s_%s_index %d\n' % (self.name, field.name, i) + result += '#define %s_%s_tag %d\n' % (self.name, field.name, field.tag) + i += 1 + return result + def iterate_messages(desc, names = Names()): '''Recursively find all messages. For each, yield name, DescriptorProto.''' if hasattr(desc, 'message_type'): @@ -383,7 +392,12 @@ def generate_header(headername, enums, messages): yield '/* Struct field encoding specification for nanopb */\n' for msg in messages: yield msg.message_declaration() + '\n' + yield '\n' + yield '/* Field indexes and tags */\n' + for msg in messages: + yield msg.field_numbers() + yield '\n#endif\n' def generate_source(headername, enums, messages): diff --git a/pb.h b/pb.h index 79b9b74..0399e53 100644 --- a/pb.h +++ b/pb.h @@ -125,6 +125,22 @@ typedef struct { */ typedef PB_MSG_STRUCT(1) pb_message_t; +/* These macros are used to manipulate the has_fields array in + * generated messages. + */ +#define PB_FIELD_INDEX(TYPE, FIELD) TYPE ## _ ## FIELD ## _index +#define PB_FIELD_BYTE(TYPE, FIELD) (PB_FIELD_INDEX(TYPE, FIELD) / 8) +#define PB_FIELD_MASK(TYPE, FIELD) (1 << (PB_FIELD_INDEX(TYPE, FIELD) & 7)) +#define PB_HAS_FIELD(STRUCT, TYPE, FIELD) \ + (((STRUCT).has_fields[PB_FIELD_BYTE(TYPE, FIELD)] \ + & PB_FIELD_MASK(TYPE, FIELD)) != 0) +#define PB_SET_FIELD(STRUCT, TYPE, FIELD) \ + ((STRUCT).has_fields[PB_FIELD_BYTE(TYPE, FIELD)] \ + |= PB_FIELD_MASK(TYPE, FIELD)) +#define PB_CLEAR_FIELD(STRUCT, TYPE, FIELD) \ + ((STRUCT).has_fields[PB_FIELD_BYTE(TYPE, FIELD)] \ + &= ~PB_FIELD_MASK(TYPE, FIELD)) + /* 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 1b0fcdd..8939966 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -251,10 +251,7 @@ static bool checkreturn decode_field(pb_istream_t *stream, pb_wire_type_t wire_t switch (PB_HTYPE(iter->current->type)) { case PB_HTYPE_REQUIRED: - return func(stream, iter->current, iter->pData); - case PB_HTYPE_OPTIONAL: - *(bool*)iter->pSize = true; return func(stream, iter->current, iter->pData); case PB_HTYPE_ARRAY: @@ -340,18 +337,17 @@ static void pb_message_set_to_defaults(const pb_message_t *msg, void *dest_struc pb_field_iterator_t iter; pb_field_init(&iter, msg, dest_struct); + /* Initialize the has_fields array to zero. */ + memset(dest_struct, 0, (msg->field_count + 7) / 8); + /* Initialize size/has fields and apply default values */ do { if (iter.field_index >= msg->field_count) continue; - /* Initialize the size field for optional/repeated fields to 0. */ - if (PB_HTYPE(iter.current->type) == PB_HTYPE_OPTIONAL) - { - *(bool*)iter.pSize = false; - } - else if (PB_HTYPE(iter.current->type) == PB_HTYPE_ARRAY) + /* Initialize the size field for repeated fields to 0. */ + if (PB_HTYPE(iter.current->type) == PB_HTYPE_ARRAY) { *(size_t*)iter.pSize = 0; continue; /* Array is empty, no need to initialize contents */ @@ -383,8 +379,8 @@ static void pb_message_set_to_defaults(const pb_message_t *msg, void *dest_struc bool checkreturn pb_decode(pb_istream_t *stream, const pb_message_t *msg, void *dest_struct) { - uint32_t fields_seen = 0; /* Used to check for required fields */ pb_field_iterator_t iter; + char *has_fields = dest_struct; unsigned int i; pb_message_set_to_defaults(msg, dest_struct); @@ -418,17 +414,17 @@ bool checkreturn pb_decode(pb_istream_t *stream, const pb_message_t *msg, void * continue; } - fields_seen |= 1 << (iter.field_index & 31); + has_fields[iter.field_index/8] |= 1 << iter.field_index%8; if (!decode_field(stream, wire_type, &iter)) return false; } - /* Check that all required fields (mod 31) were present. */ + /* Check that all required fields were present. */ for (i = 0; i < msg->field_count; i++) { if (PB_HTYPE(msg->fields[i].type) == PB_HTYPE_REQUIRED && - !(fields_seen & (1 << (i & 31)))) + !(has_fields[i/8] & 1 << (i%8))) { return false; } diff --git a/pb_encode.c b/pb_encode.c index 53f48c1..3a981d4 100644 --- a/pb_encode.c +++ b/pb_encode.c @@ -144,6 +144,7 @@ bool checkreturn pb_encode(pb_ostream_t *stream, const pb_message_t *msg, const { const void *pData = src_struct; const void *pSize; + const char *has_fields = src_struct; unsigned int i; size_t prev_size = 0; @@ -168,7 +169,7 @@ bool checkreturn pb_encode(pb_ostream_t *stream, const pb_message_t *msg, const break; case PB_HTYPE_OPTIONAL: - if (*(bool*)pSize) + if (has_fields[i/8] & (1 << i%8)) { if (!pb_encode_tag_for_field(stream, field)) return false; diff --git a/tests/encode_unittests.c b/tests/encode_unittests.c index 8608e91..2f7b7b4 100644 --- a/tests/encode_unittests.c +++ b/tests/encode_unittests.c @@ -192,7 +192,7 @@ int main() { uint8_t buffer[10]; pb_ostream_t s; - IntegerArray msg = {5, {1, 2, 3, 4, 5}}; + IntegerArray msg = {{0}, 5, {1, 2, 3, 4, 5}}; COMMENT("Test pb_encode with int32 array") @@ -208,7 +208,7 @@ int main() { uint8_t buffer[10]; pb_ostream_t s; - FloatArray msg = {1, {99.0f}}; + FloatArray msg = {{0}, 1, {99.0f}}; COMMENT("Test pb_encode with float array") @@ -236,7 +236,7 @@ int main() { uint8_t buffer[10]; pb_ostream_t s; - IntegerContainer msg = {{5, {1,2,3,4,5}}}; + IntegerContainer msg = {{0}, {{0}, 5, {1,2,3,4,5}}}; COMMENT("Test pb_encode with packed array in a submessage.") TEST(WRITES(pb_encode(&s, IntegerContainer_msg, &msg), diff --git a/tests/test_decode1.c b/tests/test_decode1.c index e68c0e6..edad6ad 100644 --- a/tests/test_decode1.c +++ b/tests/test_decode1.c @@ -25,7 +25,7 @@ bool print_person(pb_istream_t *stream) printf("name: \"%s\"\n", person.name); printf("id: %d\n", person.id); - if (person.has_email) + if (Person_has(person, email)) printf("email: \"%s\"\n", person.email); for (i = 0; i < person.phone_count; i++) diff --git a/tests/test_encode1.c b/tests/test_encode1.c index eefd9e1..5c4adf0 100644 --- a/tests/test_encode1.c +++ b/tests/test_encode1.c @@ -17,9 +17,11 @@ bool streamcallback(pb_ostream_t *stream, const uint8_t *buf, size_t 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}}}; - + Person person = {{0}, "Test Person 99", 99, "test@person.com", + 1, {{{0}, "555-12345678", Person_PhoneType_MOBILE}}}; + Person_set(person, email); + Person_PhoneNumber_set(person.phone[0], type); + /* Prepare the stream, output goes directly to stdout */ pb_ostream_t stream = {&streamcallback, stdout, SIZE_MAX, 0}; diff --git a/tests/test_encode_callbacks.c b/tests/test_encode_callbacks.c index 03a3dbf..650b80c 100644 --- a/tests/test_encode_callbacks.c +++ b/tests/test_encode_callbacks.c @@ -52,7 +52,7 @@ int main() testmessage.fixed32value.funcs.encode = &encode_fixed32; testmessage.fixed64value.funcs.encode = &encode_fixed64; - testmessage.has_submsg = true; + TestMessage_set(testmessage, submsg); testmessage.submsg.stringvalue.funcs.encode = &encode_string; testmessage.submsg.int32value.funcs.encode = &encode_int32; testmessage.submsg.fixed32value.funcs.encode = &encode_fixed32;