diff --git a/docs/concepts.rst b/docs/concepts.rst index f2b3bb2..f1c7cab 100644 --- a/docs/concepts.rst +++ b/docs/concepts.rst @@ -142,6 +142,7 @@ Most Protocol Buffers datatypes have directly corresponding C datatypes, such as 1) Strings, bytes and repeated fields of any type map to callback functions by default. 2) If there is a special option *(nanopb).max_size* specified in the .proto file, string maps to null-terminated char array and bytes map to a structure containing a char array and a size field. 3) If there is a special option *(nanopb).max_count* specified on a repeated field, it maps to an array of whatever type is being repeated. Another field will be created for the actual number of entries stored. +4) The option *(nanopb).pointer* can override the default (false, unless the *-p* option is passed to *nanopb_generator.py*) behavior. If a string, byte, or submessage is generated as a pointer field, and it is repeated (with a maximum count), required, or optional, the members will be pointers rather than in-line data. =============================================================================== ======================= field in .proto autogenerated in .h @@ -156,10 +157,19 @@ required bytes data = 1 [(nanopb).max_size = 40]; | uint8_t bytes[40]; | } Person_data_t; | Person_data_t data; +required string name = 1 [(nanopb).pointer = true]; char \*name; +required bytes data = 1 [(nanopb).pointer = true]; pb_bytes_t data; +required bytes data = 1 [(nanopb).pointer = true, (nanopb).max_count = 5]; | size_t data_count; + | pb_bytes_t data[5]; +required Message msg = 1 [(nanopb).pointer = true]; Message \*msg; =============================================================================== ======================= The maximum lengths are checked in runtime. If string/bytes/array exceeds the allocated length, *pb_decode* will return false. +If a pointer-type field is not received, the field will be marked as absent, but the pointer will not be modified. This helps reduce memory fragmentation and churn, but increases worst-case memory usage and means you must use the *Message_has(object, Field)* macro rather than testing for a null pointer. You can use the `pb_clean`_ function to release unused memory in these cases. + +.. _`pb_clean`: reference.html#pb-clean + Field callbacks =============== When a field has dynamic length, nanopb cannot statically allocate storage for it. Instead, it allows you to handle the field in whatever way you want, using a callback function. @@ -279,6 +289,8 @@ 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. +.. Should there be a section here on pointer fields? + Return values and error handling ================================ diff --git a/docs/index.rst b/docs/index.rst index 5365600..fa7469f 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -15,6 +15,8 @@ Overall structure For the runtime program, you always need *pb.h* for type declarations. Depending on whether you want to encode, decode, or both, you also need *pb_encode.h/c* or *pb_decode.h/c*. +If your *.proto* file encodes submessages or other fields using pointers, you must compile *pb_decode.c* with a preprocessor macro named *MALLOC_HEADER* that is the name of a header with definitions (either as functions or macros) for *malloc()*, *realloc()* and *free()*. For a typical hosted configuration, this should be **. + The high-level encoding and decoding functions take an array of *pb_field_t* structures, which describes the fields of a message structure. Usually you want these autogenerated from a *.proto* file. The tool script *nanopb_generator.py* accomplishes this. .. image:: generator_flow.png @@ -52,7 +54,7 @@ Features and limitations #) Unknown fields are not preserved when decoding and re-encoding a message. #) Reflection (runtime introspection) is not supported. E.g. you can't request a field by giving its name in a string. #) Numeric arrays are always encoded as packed, even if not marked as packed in .proto. This causes incompatibility with decoders that do not support packed format. -#) Cyclic references between messages are not supported. They could be supported in callback-mode if there was an option in the generator to set the mode. +#) Limited support for cyclic references between messages. (The cycle must be broken by making one of the references a pointer or callback field, and objects that have circular references are not detected when encoding.) Getting started =============== diff --git a/docs/reference.rst b/docs/reference.rst index 6fd2b3c..f481881 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -28,7 +28,7 @@ PB_LTYPE_STRING 0x04 Null-terminated string. PB_LTYPE_SUBMESSAGE 0x05 Submessage structure. ==================== ===== ================================================ -The high-order nibble defines whether the field is required, optional, repeated or callback: +The high-order nibble defines whether the field is required, optional, repeated or callback, and whether it is a pointer: ==================== ===== ================================================ HTYPE identifier Value Field handling @@ -41,6 +41,10 @@ PB_HTYPE_ARRAY 0x20 A repeated field with preallocated array. PB_HTYPE_CALLBACK 0x30 A field with dynamic storage size, data is actually a pointer to a structure containing a callback function. +PB_HTYPE_POINTER 0x80 For required, optional and array non-scalar + fields, uses a pointer type (char* for strings, + pb_bytes_t for bytes, or a pointer for + submessages). ==================== ===== ================================================ pb_field_t @@ -79,6 +83,16 @@ An byte array with a field for storing the length:: In an actual array, the length of *bytes* may be different. +pb_bytes_t +---------- +A byte array with fields for storing the allocated and current lengths:: + + typedef struct { + size_t alloced; + size_t size; + uint8_t *bytes; + } pb_bytes_array_t; + pb_callback_t ------------- Part of a message structure, for fields with type PB_HTYPE_CALLBACK:: @@ -368,6 +382,22 @@ In addition to EOF, the pb_decode implementation supports terminating a message For optional fields, this function applies the default value and clears the corresponding bit in *has_fields* if the field is not present. +pb_clean +-------- +Release and clear all the unused pointers of a structure. :: + + bool pb_clean(const pb_message_t *msg, void *dest_struct); + +:msg: A message descriptor. Usually autogenerated. +:dest_struct: Pointer to structure to be cleaned. +:returns: True on success, false if one of the unused message fields has a pointer type that this function cannot handle. + +For each string or submessage with pointer type, this function calls *free()* on the pointer and sets the pointer to NULL. + +For bytes fields with pointer type, this function calls *free()* on the allocated data, nulls that pointer and zeros the size fields in the generated pb_bytes_t structure. + +For repeated fields, it applies the corresponding operation above to each unused element of the generated array. + .. sidebar:: Field decoders The functions with names beginning with *pb_dec_* are called field decoders. Each PB_LTYPE has an own field decoder, which handles translating from Protocol Buffers data to C data. diff --git a/generator/nanopb.proto b/generator/nanopb.proto index 2610cd5..f8a4b31 100644 --- a/generator/nanopb.proto +++ b/generator/nanopb.proto @@ -1,6 +1,7 @@ // Custom options for defining: // - Maximum size of string/bytes // - Maximum number of elements in array +// - Pointer or in-line representation of non-scalar fields // // These are used by nanopb to generate statically allocable structures // for memory-limited environments. @@ -10,6 +11,7 @@ import "google/protobuf/descriptor.proto"; message NanoPBOptions { optional int32 max_size = 1; optional int32 max_count = 2; + optional bool pointer = 3; } // Protocol Buffers extension number registry diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index 9e21672..63c95ef 100644 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -3,6 +3,7 @@ import google.protobuf.descriptor_pb2 as descriptor import nanopb_pb2 import os.path +from optparse import OptionParser # Values are tuple (c type, pb ltype) FieldD = descriptor.FieldDescriptorProto @@ -21,6 +22,12 @@ datatypes = { FieldD.TYPE_UINT32: ('uint32_t', 'PB_LTYPE_VARINT'), FieldD.TYPE_UINT64: ('uint64_t', 'PB_LTYPE_VARINT') } +pointable_types = frozenset([ + FieldD.TYPE_STRING, + FieldD.TYPE_BYTES, + FieldD.TYPE_MESSAGE +]) +options = None class Names: '''Keeps a set of nested names and formats them to C identifier. @@ -70,6 +77,8 @@ class Field: self.max_size = None self.max_count = None self.array_decl = "" + self.is_pointer = options.pointer + self.is_array = False # Parse nanopb-specific field options if desc.options.HasExtension(nanopb_pb2.nanopb): @@ -78,6 +87,11 @@ class Field: self.max_size = ext.max_size if ext.HasField("max_count"): self.max_count = ext.max_count + if ext.HasField("pointer"): + self.is_pointer = ext.pointer + if desc.type not in pointable_types: + raise NotImplementedError('Cannot make %s.%s a pointer' + % (struct_name, desc.name)) if desc.HasField('default_value'): self.default = desc.default_value @@ -96,8 +110,11 @@ class Field: else: self.htype = 'PB_HTYPE_ARRAY' self.array_decl = '[%d]' % self.max_count + self.is_array = True else: raise NotImplementedError(desc.label) + if self.is_pointer: + self.htype += ' | PB_HTYPE_POINTER' # Decide LTYPE and CTYPE # LTYPE is the low-order nibble of nanopb field description, @@ -112,14 +129,18 @@ class Field: self.default = self.ctype + self.default elif desc.type == FieldD.TYPE_STRING: self.ltype = 'PB_LTYPE_STRING' - if self.max_size is None: + if self.is_pointer: + self.ctype = 'char' + elif self.max_size is None: is_callback = True else: self.ctype = 'char' self.array_decl += '[%d]' % self.max_size elif desc.type == FieldD.TYPE_BYTES: self.ltype = 'PB_LTYPE_BYTES' - if self.max_size is None: + if self.is_pointer: + self.ctype = 'pb_bytes_t' + elif self.max_size is None: is_callback = True else: self.ctype = self.struct_name + self.name + 't' @@ -138,11 +159,14 @@ class Field: return cmp(self.tag, other.tag) def __str__(self): - if self.htype == 'PB_HTYPE_ARRAY': + if self.is_array: result = ' size_t ' + self.name + '_count;\n' else: result = '' - result += ' %s %s%s;' % (self.ctype, self.name, self.array_decl) + if self.is_pointer and self.ctype != 'pb_bytes_t': + result += ' %s *%s%s;' % (self.ctype, self.name, self.array_decl) + else: + result += ' %s %s%s;' % (self.ctype, self.name, self.array_decl) return result def types(self): @@ -205,17 +229,16 @@ class Field: else: result += ' pb_delta_end(%s, %s, %s),' % (self.struct_name, self.name, prev_field_name) - if self.htype == 'PB_HTYPE_ARRAY': + if self.is_array: result += '\n pb_delta(%s, %s_count, %s),' % (self.struct_name, self.name, self.name) else: result += ' 0,' - - if self.htype == 'PB_HTYPE_ARRAY': + if self.is_array: 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.htype != 'PB_HTYPE_CALLBACK' and self.ltype == 'PB_LTYPE_BYTES': + elif self.htype != 'PB_HTYPE_CALLBACK' and not self.is_pointer and self.ltype == 'PB_LTYPE_BYTES': result += '\n pb_membersize(%s, bytes),' % self.ctype result += ' 0,' else: @@ -240,13 +263,14 @@ class Message: def get_dependencies(self): '''Get list of type names that this structure refers to.''' - return [str(field.ctype) for field in self.fields] + return [str(field.ctype) for field in self.fields if not + (field.is_pointer or field.htype == 'PB_HTYPE_CALLBACK')] def __str__(self): - result = 'typedef struct {\n' + result = 'struct %s {\n' % self.name 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 + result += '\n};' return result def types(self): @@ -257,6 +281,9 @@ class Message: result += types + '\n' return result + def typedef(self): + return 'typedef struct %s %s;' % (self.name, self.name) + def default_decl(self, declaration_only = False): result = "" for field in self.fields: @@ -273,7 +300,7 @@ class Message: def message_definition(self): result = 'const %s_msg_t %s_real_msg = {\n' % (self.name, self.name) - result += ' %d,\n' % len(self.fields) + result += ' %d, sizeof(%s),\n' % (len(self.fields), self.name) result += ' {\n\n' prev = None @@ -379,6 +406,11 @@ def generate_header(headername, enums, messages): for enum in enums: yield str(enum) + '\n\n' + yield '/* Struct typedefs */\n' + for msg in messages: + yield msg.typedef() + '\n' + yield '\n' + yield '/* Struct definitions */\n' for msg in sort_dependencies(messages): yield msg.types() @@ -418,18 +450,25 @@ if __name__ == '__main__': import sys import os.path - if len(sys.argv) != 2: - print "Usage: " + sys.argv[0] + " file.pb" - print "where file.pb has been compiled from .proto by:" - print "protoc -ofile.pb file.proto" - print "Output fill be written to file.pb.h and file.pb.c" - sys.exit(1) - - data = open(sys.argv[1]).read() + parser = OptionParser(usage="Usage: %prog [options] file.pb", + epilog= +"""file.pb should be compiled from file.proto by: +protoc -ofile.pb file.proto +""") + parser.add_option("-p", "--pointer", dest="pointer", + action="store_true", default=False, + help="generate pointers for non-scalar fields") + + (options, args) = parser.parse_args() + + if len(args) != 1: + parser.error("Please provide exactly one file.pb argument") + + data = open(args[0]).read() fdesc = descriptor.FileDescriptorSet.FromString(data) enums, messages = parse_file(fdesc.file[0]) - noext = os.path.splitext(sys.argv[1])[0] + noext = os.path.splitext(args[0])[0] headername = noext + '.pb.h' sourcename = noext + '.pb.c' headerbasename = os.path.basename(headername) diff --git a/generator/nanopb_pb2.py b/generator/nanopb_pb2.py index f2fbeef..78b6985 100644 --- a/generator/nanopb_pb2.py +++ b/generator/nanopb_pb2.py @@ -7,10 +7,12 @@ from google.protobuf import descriptor_pb2 # @@protoc_insertion_point(imports) +import google.protobuf.descriptor_pb2 + DESCRIPTOR = descriptor.FileDescriptor( name='nanopb.proto', package='', - serialized_pb='\n\x0cnanopb.proto\x1a google/protobuf/descriptor.proto\"4\n\rNanoPBOptions\x12\x10\n\x08max_size\x18\x01 \x01(\x05\x12\x11\n\tmax_count\x18\x02 \x01(\x05:>\n\x06nanopb\x12\x1d.google.protobuf.FieldOptions\x18\xf2\x07 \x01(\x0b\x32\x0e.NanoPBOptions') + serialized_pb='\n\x0cnanopb.proto\x1a google/protobuf/descriptor.proto\"E\n\rNanoPBOptions\x12\x10\n\x08max_size\x18\x01 \x01(\x05\x12\x11\n\tmax_count\x18\x02 \x01(\x05\x12\x0f\n\x07pointer\x18\x03 \x01(\x08:>\n\x06nanopb\x12\x1d.google.protobuf.FieldOptions\x18\xf2\x07 \x01(\x0b\x32\x0e.NanoPBOptions') NANOPB_FIELD_NUMBER = 1010 @@ -44,6 +46,13 @@ _NANOPBOPTIONS = descriptor.Descriptor( message_type=None, enum_type=None, containing_type=None, is_extension=False, extension_scope=None, options=None), + descriptor.FieldDescriptor( + name='pointer', full_name='NanoPBOptions.pointer', index=2, + number=3, type=8, cpp_type=7, label=1, + has_default_value=False, default_value=False, + message_type=None, enum_type=None, containing_type=None, + is_extension=False, extension_scope=None, + options=None), ], extensions=[ ], @@ -54,11 +63,10 @@ _NANOPBOPTIONS = descriptor.Descriptor( is_extendable=False, extension_ranges=[], serialized_start=50, - serialized_end=102, + serialized_end=119, ) -import google.protobuf.descriptor_pb2 - +DESCRIPTOR.message_types_by_name['NanoPBOptions'] = _NANOPBOPTIONS class NanoPBOptions(message.Message): __metaclass__ = reflection.GeneratedProtocolMessageType diff --git a/pb.h b/pb.h index 0399e53..9ad9adc 100644 --- a/pb.h +++ b/pb.h @@ -78,7 +78,14 @@ typedef uint8_t pb_type_t; * used to speculatively index an array). */ #define PB_HTYPE_CALLBACK 0x30 -#define PB_HTYPE(x) ((x) & 0xF0) +/* Indicates that a string, bytes or non-repeated submessage is + * represented using a pointer (char* for string, pb_byptes_array_t + * for bytes). + */ +#define PB_HTYPE_POINTER 0x80 + +#define PB_POINTER(x) ((x) & PB_HTYPE_POINTER) +#define PB_HTYPE(x) ((x) & 0x70) #define PB_LTYPE(x) ((x) & 0x0F) /* This structure is used in auto-generated constants @@ -97,7 +104,7 @@ struct _pb_field_t { uint8_t data_size; /* Data size in bytes for a single item */ uint8_t array_size; /* Maximum number of entries in array */ - /* Field definitions for submessage + /* Pointer to message structure for submessage * OR default value for all other non-array, non-callback types * If null, then field will zeroed. */ const void *ptr; @@ -112,11 +119,20 @@ typedef struct { uint8_t bytes[1]; } pb_bytes_array_t; -/* This macro is define the type of a structure for a message with N +/* This structure is used for dynamically allocated 'bytes' arrays. + */ +typedef struct { + size_t size; + size_t alloced; + uint8_t *bytes; +} pb_bytes_t; + +/* This macro defines the type of a structure for a message with N * fields. */ #define PB_MSG_STRUCT(N) struct { \ unsigned int field_count; \ + size_t size; \ pb_field_t fields[N]; \ } diff --git a/pb_decode.c b/pb_decode.c index 8939966..232c7b0 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -14,6 +14,9 @@ #include "pb.h" #include "pb_decode.h" #include +#ifdef MALLOC_HEADER +# include MALLOC_HEADER +#endif typedef bool (*pb_decoder_t)(pb_istream_t *stream, const pb_field_t *field, void *dest) checkreturn; @@ -360,6 +363,9 @@ static void pb_message_set_to_defaults(const pb_message_t *msg, void *dest_struc } else if (PB_LTYPE(iter.current->type) == PB_LTYPE_SUBMESSAGE) { + if (PB_POINTER(iter.current->type) && + (*(void**)iter.pData == NULL)) + continue; pb_message_set_to_defaults(iter.current->ptr, iter.pData); } else if (iter.current->ptr != NULL) @@ -433,6 +439,85 @@ bool checkreturn pb_decode(pb_istream_t *stream, const pb_message_t *msg, void * return true; } +#ifdef MALLOC_HEADER + +/* Clean a single unused field (or unused array element). */ +static bool checkreturn pb_clean_pointer(const pb_field_t *field, void *data) +{ + switch (PB_LTYPE(field->type)) + { + case PB_LTYPE_BYTES: + { + pb_bytes_t *bytes = (pb_bytes_t*)data; + bytes->size = 0; + bytes->alloced = 0; + free(bytes->bytes); + return true; + } + case PB_LTYPE_STRING: + case PB_LTYPE_SUBMESSAGE: + { + void **obj = (void**)data; + free(*obj); + *obj = NULL; + return true; + } + default: + return false; + } +} + +/* Clean unused trailing elements in an array. */ +static bool checkreturn pb_clean_array(const pb_field_t *field, void *data, size_t count) +{ + unsigned int i; + for (i = count; i < field->array_size; i++) + { + if (!pb_clean_pointer(field, data)) + return false; + data = (char*)data + field->data_size; + } + return true; +} + +/* Clean unused fields in a message. */ +bool pb_clean(const pb_message_t *msg, void *dest_struct) +{ + const char *has_fields = dest_struct; + const void *pSize; + char *pData = (char*)dest_struct; + size_t prev_size = 0; + unsigned int i; + + for (i = 0; i < msg->field_count; i++) + { + const pb_field_t *field = &msg->fields[i]; + bool status; + pData = pData + prev_size + field->data_offset; + pSize = pData + field->size_offset; + prev_size = field->data_size; + if (PB_HTYPE(field->type) == PB_HTYPE_ARRAY) + prev_size *= field->array_size; + + if (!PB_POINTER(field->type)) + continue; + else if (PB_HTYPE(field->type) == PB_HTYPE_ARRAY) + status = pb_clean_array(field, pData, *(size_t*)pSize); + else if (!(has_fields[i/8] & 1 << (i%8))) + status = pb_clean_pointer(field, pData); + else /* pointer field is used */ + continue; + + if (!status) + return false; + } + + return true; +} + +#endif + + /* Field decoders */ /* Copy destsize bytes from src so that values are casted properly. @@ -505,6 +590,24 @@ bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, voi if (x->size > field->data_size) return false; +#ifdef MALLOC_HEADER + if (PB_POINTER(field->type)) + { + pb_bytes_t *x2 = (pb_bytes_t*)dest; + + if (x2->alloced < x2->size) + { + void *new_bytes = realloc(x2->bytes, x2->size); + if (!new_bytes) + return false; + x2->alloced = x2->size; + x2->bytes = new_bytes; + } + + return pb_read(stream, x2->bytes, x2->size); + } +#endif + return pb_read(stream, x->bytes, x->size); } @@ -518,6 +621,18 @@ bool checkreturn pb_dec_string(pb_istream_t *stream, const pb_field_t *field, vo if (size > field->data_size - 1) return false; +#ifdef MALLOC_HEADER + if (PB_POINTER(field->type)) + { + uint8_t *string = (uint8_t*)realloc(*(uint8_t**)dest, size + 1); + if (!string) + return false; + + *(uint8_t**)dest = string; + dest = string; + } +#endif + status = pb_read(stream, (uint8_t*)dest, size); *((uint8_t*)dest + size) = 0; return status; @@ -527,6 +642,7 @@ bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t *field { bool status; pb_istream_t substream; + const pb_message_t *msg; if (!make_string_substream(stream, &substream)) return false; @@ -534,7 +650,25 @@ bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t *field if (field->ptr == NULL) return false; - status = pb_decode(&substream, (const pb_message_t*)field->ptr, dest); + msg = (const pb_message_t*)field->ptr; + +#ifdef MALLOC_HEADER + if (PB_POINTER(field->type)) + { + if (*(void**)dest == NULL) + { + void *object = malloc(msg->size); + if (!object) + return false; + *(void**)dest = object; + dest = object; + } else { + dest = *(void**)dest; + } + } +#endif + + status = pb_decode(&substream, msg, dest); stream->state = substream.state; return status; } diff --git a/pb_decode.h b/pb_decode.h index 0304bd1..c3642f2 100644 --- a/pb_decode.h +++ b/pb_decode.h @@ -68,4 +68,10 @@ bool pb_dec_bytes(pb_istream_t *stream, const pb_field_t *field, void *dest); bool pb_dec_string(pb_istream_t *stream, const pb_field_t *field, void *dest); bool pb_dec_submessage(pb_istream_t *stream, const pb_field_t *field, void *dest); +/* Release memory and clear pointers for any unused elements of + * dest_struct. This function is only compiled when MALLOC_HEADER is + * defined for pb_decode.c. + */ +bool pb_clean(const pb_message_t *msg, void *dest_struct); + #endif diff --git a/pb_encode.c b/pb_encode.c index 3a981d4..6837f32 100644 --- a/pb_encode.c +++ b/pb_encode.c @@ -161,6 +161,16 @@ bool checkreturn pb_encode(pb_ostream_t *stream, const pb_message_t *msg, const switch (PB_HTYPE(field->type)) { + case PB_HTYPE_OPTIONAL: + if (!(has_fields[i/8] & (1 << i%8))) + break; + if (PB_POINTER(field->type) + && (PB_LTYPE(field->type) == PB_LTYPE_STRING + || PB_LTYPE(field->type) == PB_LTYPE_SUBMESSAGE) + && *(void**)pData == NULL) + break; + /* else fall through to required case */ + case PB_HTYPE_REQUIRED: if (!pb_encode_tag_for_field(stream, field)) return false; @@ -168,17 +178,6 @@ bool checkreturn pb_encode(pb_ostream_t *stream, const pb_message_t *msg, const return false; break; - case PB_HTYPE_OPTIONAL: - if (has_fields[i/8] & (1 << i%8)) - { - if (!pb_encode_tag_for_field(stream, field)) - return false; - - if (!func(stream, field, pData)) - return false; - } - break; - case PB_HTYPE_ARRAY: if (!encode_array(stream, field, pData, *(size_t*)pSize, func)) return false; @@ -334,13 +333,22 @@ 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) { - pb_bytes_array_t *bytes = (pb_bytes_array_t*)src; - return pb_encode_string(stream, bytes->bytes, bytes->size); + if ((field != NULL) && PB_POINTER(field->type)) { + pb_bytes_t *bytes = (pb_bytes_t*)src; + return pb_encode_string(stream, bytes->bytes, bytes->size); + } else { + pb_bytes_array_t *bytes = (pb_bytes_array_t*)src; + 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) { - return pb_encode_string(stream, (uint8_t*)src, strlen((char*)src)); + size_t len; + if ((field != NULL) && PB_POINTER(field->type)) + src = *(char**)src; + len = src ? strlen((char*)src) : 0; + return pb_encode_string(stream, (uint8_t*)src, len); } bool checkreturn pb_enc_submessage(pb_ostream_t *stream, const pb_field_t *field, const void *src) @@ -352,6 +360,11 @@ bool checkreturn pb_enc_submessage(pb_ostream_t *stream, const pb_field_t *field if (field->ptr == NULL) return false; + if (PB_POINTER(field->type)) { + src = *(void**)src; + if (src == NULL) + return false; + } if (!pb_encode(&substream, (const pb_message_t*)field->ptr, src)) return false; diff --git a/tests/Makefile b/tests/Makefile index 2d5ee50..229b6a9 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,12 +1,12 @@ CFLAGS=-ansi -Wall -Werror -I .. -g -O0 --coverage LDFLAGS=--coverage DEPS=../pb_decode.h ../pb_encode.h ../pb.h person.pb.h callbacks.pb.h unittests.h unittestproto.pb.h -TESTS=test_decode1 test_encode1 decode_unittests encode_unittests +TESTS=test_decode1 test_encode1 test_decode_callbacks test_encode_callbacks decode_unittests decode_ptr_unittests encode_unittests all: breakpoints $(TESTS) run_unittests clean: - rm -f $(TESTS) person.pb* *.o *.gcda *.gcno + rm -f $(TESTS) *.pb.c *.pb.h *.o *.gcda *.gcno %.o: %.c %.o: %.c $(DEPS) @@ -16,12 +16,18 @@ pb_encode.o: ../pb_encode.c $(DEPS) $(CC) $(CFLAGS) -c -o $@ $< pb_decode.o: ../pb_decode.c $(DEPS) $(CC) $(CFLAGS) -c -o $@ $< +pb_ptr_decode.o: ../pb_decode.c $(DEPS) + $(CC) $(CFLAGS) -c -o $@ $< +decode_ptr_unittests.o: decode_unittests.c $(DEPS) + $(CC) $(CFLAGS) -c -o $@ $< test_decode1: test_decode1.o pb_decode.o person.pb.o test_encode1: test_encode1.o pb_encode.o person.pb.o test_decode_callbacks: test_decode_callbacks.o pb_decode.o callbacks.pb.o test_encode_callbacks: test_encode_callbacks.o pb_encode.o callbacks.pb.o decode_unittests: decode_unittests.o pb_decode.o unittestproto.pb.o +pb_ptr_decode.o decode_ptr_unittests.o: CFLAGS += -DMALLOC_HEADER="" +decode_ptr_unittests: decode_ptr_unittests.o pb_ptr_decode.o unittestproto.pb.o encode_unittests: encode_unittests.o pb_encode.o unittestproto.pb.o %.pb: %.proto @@ -37,10 +43,11 @@ coverage: run_unittests gcov pb_encode.gcda gcov pb_decode.gcda -run_unittests: decode_unittests encode_unittests test_encode1 test_decode1 test_encode_callbacks test_decode_callbacks +run_unittests: decode_unittests decode_ptr_unittests encode_unittests test_encode1 test_decode1 test_encode_callbacks test_decode_callbacks rm -f *.gcda ./decode_unittests > /dev/null + ./decode_ptr_unittests > /dev/null ./encode_unittests > /dev/null [ "`./test_encode1 | ./test_decode1`" = \ diff --git a/tests/decode_unittests.c b/tests/decode_unittests.c index 5f95f57..b227fdc 100644 --- a/tests/decode_unittests.c +++ b/tests/decode_unittests.c @@ -279,6 +279,30 @@ int main() TEST((s = S("\x08"), !pb_decode(&s, IntegerArray_msg, &dest))) } +#ifdef MALLOC_HEADER + { + pb_istream_t s; + PointerContainer dest; + + COMMENT("Testing pb_decode with pointer fields") + + memset(&dest, 0, sizeof(dest)); + TEST((s = S("\x0A\x01\x61\x12\x01\x62\x2A\x01\x65\x32\x01\x66\x3A\x00" + "\x42\x01\x63\x4A\x01\x64"), + pb_decode(&s, PointerContainer_msg, &dest))) + TEST(0 == strcmp(dest.text, "a")) + TEST(dest.blob.size == 1 && dest.blob.bytes[0] == 'b') + TEST(dest.submsg == NULL) + TEST(dest.rtext_count == 1 && (0 == strcmp(dest.rtext[0], "e"))) + TEST(dest.rblob_count == 1 && dest.rblob[0].size == 1 && + dest.rblob[0].bytes[0] == 'f') + TEST(dest.rsubmsg_count == 1) + TEST(0 == strcmp(dest.otext, "c")) + TEST(dest.oblob.size == 1 && dest.oblob.bytes[0] == 'd') + TEST(pb_clean(PointerContainer_msg, &dest)); + } +#endif + if (status != 0) fprintf(stdout, "\n\nSome tests FAILED!\n"); diff --git a/tests/encode_unittests.c b/tests/encode_unittests.c index 2f7b7b4..d3b3151 100644 --- a/tests/encode_unittests.c +++ b/tests/encode_unittests.c @@ -269,6 +269,47 @@ int main() TEST(!pb_encode(&s, CallbackContainerContainer_msg, &msg2)) } + { + uint8_t buffer[128]; + pb_ostream_t s; + PointerContainer msg; + PointerContainer msg2; + IntegerArray msg3; + + COMMENT("Test pb_encode with pointer fields.") + + memset(&msg, 0, sizeof(msg)); + memset(&msg2, 0, sizeof(msg2)); + msg.text = "a"; + msg.blob.size = 1; + msg.blob.bytes = (uint8_t*)"b"; + msg.submsg = &msg2; + TEST(WRITES(pb_encode(&s, PointerContainer_msg, &msg), + "\x0A\x01\x61\x12\x01\x62")) + + memset(&msg3, 0, sizeof(msg3)); + msg.rtext_count = 1; + msg.rtext[0] = "e"; + msg.rblob_count = 1; + msg.rblob[0].size = 1; + msg.rblob[0].bytes = (uint8_t*)"f"; + msg.rsubmsg_count = 1; + msg.rsubmsg[0] = &msg3; + TEST(WRITES(pb_encode(&s, PointerContainer_msg, &msg), + "\x0A\x01\x61\x12\x01\x62" + "\x2A\x01\x65\x32\x01\x66\x3A\x00")); + + PointerContainer_set(msg, otext); + msg.otext = "c"; + PointerContainer_set(msg, oblob); + msg.oblob.size = 1; + msg.oblob.bytes = (uint8_t*)"d"; + TEST(WRITES(pb_encode(&s, PointerContainer_msg, &msg), + "\x0A\x01\x61\x12\x01\x62" + "\x2A\x01\x65\x32\x01\x66\x3A\x00" + "\x42\x01\x63\x4A\x01\x64")); + } + if (status != 0) fprintf(stdout, "\n\nSome tests FAILED!\n"); diff --git a/tests/unittestproto.proto b/tests/unittestproto.proto index c8a39dd..89d571e 100644 --- a/tests/unittestproto.proto +++ b/tests/unittestproto.proto @@ -26,3 +26,24 @@ message CallbackContainer { message CallbackContainerContainer { required CallbackContainer submsg = 1; } + +message PointerContainer { + required string text = 1 [(nanopb).pointer = true]; + required bytes blob = 2 [(nanopb).pointer = true]; + optional PointerContainer submsg = 3 [(nanopb).pointer = true]; + // This should be rejected: + // required int32 data = 4 [(nanopb).pointer = true]; + repeated string rtext = 5 [(nanopb).pointer = true, (nanopb).max_count = 10]; + repeated bytes rblob = 6 [(nanopb).pointer = true, (nanopb).max_count = 10]; + repeated IntegerArray rsubmsg = 7 [(nanopb).pointer = true, (nanopb).max_count = 10]; + optional string otext = 8 [(nanopb).pointer = true]; + optional bytes oblob = 9 [(nanopb).pointer = true]; +} + +message RecursiveRef_A { + optional RecursiveRef_B submsg = 1 [(nanopb).pointer = true]; +} + +message RecursiveRef_B { + required RecursiveRef_A submsg = 1; +}