From 62afd54964528c1fbd5ab802134f7e9ad912d904 Mon Sep 17 00:00:00 2001 From: Tom Roeder Date: Tue, 2 Aug 2016 14:57:37 -0700 Subject: [PATCH 1/1] Add inline allocation of bytes fields This commit adds a new FT_INLINE allocation type that forces bytes fields to be inlined into the struct. E.g., pb_byte_t my_bytes[32]. This requires max_size for the bytes field. The FT_INLINE type is represented as a new LTYPE: FT_LTYPE_FIXED_LENGTH_BYTES. This commit also updates the documentation with FT_INLINE and FT_LTYPE_FIXED_LENGTH_BYTES. Added an AUTHORS file in apparent order of appearance in the git log history from $(git log --all). --- AUTHORS | 24 ++++++++++++++ docs/concepts.rst | 4 ++- docs/index.rst | 4 +-- docs/reference.rst | 49 ++++++++++++++------------- generator/nanopb_generator.py | 35 ++++++++++++++++---- generator/proto/nanopb.proto | 1 + pb.h | 27 +++++++++++++-- pb_decode.c | 9 ++++- pb_encode.c | 15 ++++++--- tests/inline/SConscript | 16 +++++++++ tests/inline/inline.expected | 3 ++ tests/inline/inline.proto | 17 ++++++++++ tests/inline/inline_unittests.c | 73 +++++++++++++++++++++++++++++++++++++++++ 13 files changed, 239 insertions(+), 38 deletions(-) create mode 100644 AUTHORS create mode 100644 tests/inline/SConscript create mode 100644 tests/inline/inline.expected create mode 100644 tests/inline/inline.proto create mode 100644 tests/inline/inline_unittests.c diff --git a/AUTHORS b/AUTHORS new file mode 100644 index 00000000..a967d3ec --- /dev/null +++ b/AUTHORS @@ -0,0 +1,24 @@ +Petteri Aimonen +Michael Poole +Daniel Kan +Stan Hu +dch +Steffen Siering +Jens Steinhauser +Pavel Ilin +Kent Ryhorchuk +Martin Donath +Oliver Lee +Michael Haberler +Nicolas Colomer +Ivan Kravets +Kyle Manna +Benjamin Kamath +Andrew Ruder +Kenshi Kawaguchi +isotes +Maxim Khitrov +Yaniv Mordekhay +Ming Zhao +Google, Inc. +Tom Roeder diff --git a/docs/concepts.rst b/docs/concepts.rst index b4f657e2..c43d8299 100644 --- a/docs/concepts.rst +++ b/docs/concepts.rst @@ -148,6 +148,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 *(nanopb).type* is set to *FT_INLINE* and *(nanopb).max_size* is also set, then bytes map to an inline byte array of fixed size. 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. =============================================================================== ======================= @@ -160,9 +161,10 @@ repeated string name = 1 [(nanopb).max_size = 40, (nanopb).max_count = 5]; | char name[5][40]; required bytes data = 1 [(nanopb).max_size = 40]; | typedef struct { | size_t size; - | uint8_t bytes[40]; + | pb_byte_t bytes[40]; | } Person_data_t; | Person_data_t data; +required bytes data = 1 [(nanopb).max_size = 40, (nanopb.type) = FT_INLINE]; | pb_byte_t data[40]; =============================================================================== ======================= The maximum lengths are checked in runtime. If string/bytes/array exceeds the allocated length, *pb_decode* will return false. diff --git a/docs/index.rst b/docs/index.rst index 24328574..afc7ee4f 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -7,7 +7,7 @@ Nanopb: Protocol Buffers with small code size Nanopb is an ANSI-C library for encoding and decoding messages in Google's `Protocol Buffers`__ format with minimal requirements for RAM and code space. It is primarily suitable for 32-bit microcontrollers. -__ http://code.google.com/apis/protocolbuffers/ +__ https://developers.google.com/protocol-buffers/docs/reference/overview Overall structure ================= @@ -54,7 +54,7 @@ Features and limitations #) Fields in the generated structs are ordered by the tag number, instead of the natural ordering in .proto file. #) 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.. +#) Numeric arrays are always encoded as packed, even if not marked as packed in .proto. #) Cyclic references between messages are supported only in callback and malloc mode. Getting started diff --git a/docs/reference.rst b/docs/reference.rst index 7d27a515..ef3867a1 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -77,9 +77,11 @@ int_size Override the integer type of a field. type Type of the generated field. Default value is *FT_DEFAULT*, which selects automatically. You can use *FT_CALLBACK*, *FT_POINTER*, - *FT_STATIC* or *FT_IGNORE* to force a callback - field, a dynamically allocated field, a static - field or to completely ignore the field. + *FT_STATIC*, *FT_IGNORE*, or *FT_INLINE* to + force a callback field, a dynamically + allocated field, a static field, to + completely ignore the field or to + generate an inline bytes field. long_names Prefix the enum name to the enum value in definitions, i.e. *EnumName_EnumValue*. Enabled by default. @@ -216,17 +218,20 @@ Type used to store the type of each field, to control the encoder/decoder behavi The low-order nibble of the enumeration values defines the function that can be used for encoding and decoding the field data: -==================== ===== ================================================ -LTYPE identifier Value Storage format -==================== ===== ================================================ -PB_LTYPE_VARINT 0x00 Integer. -PB_LTYPE_SVARINT 0x01 Integer, zigzag encoded. -PB_LTYPE_FIXED32 0x02 32-bit integer or floating point. -PB_LTYPE_FIXED64 0x03 64-bit integer or floating point. -PB_LTYPE_BYTES 0x04 Structure with *size_t* field and byte array. -PB_LTYPE_STRING 0x05 Null-terminated string. -PB_LTYPE_SUBMESSAGE 0x06 Submessage structure. -==================== ===== ================================================ +=========================== ===== ================================================ +LTYPE identifier Value Storage format +=========================== ===== ================================================ +PB_LTYPE_VARINT 0x00 Integer. +PB_LTYPE_UVARINT 0x01 Unsigned integer. +PB_LTYPE_SVARINT 0x02 Integer, zigzag encoded. +PB_LTYPE_FIXED32 0x03 32-bit integer or floating point. +PB_LTYPE_FIXED64 0x04 64-bit integer or floating point. +PB_LTYPE_BYTES 0x05 Structure with *size_t* field and byte array. +PB_LTYPE_STRING 0x06 Null-terminated string. +PB_LTYPE_SUBMESSAGE 0x07 Submessage structure. +PB_LTYPE_EXTENSION 0x08 Point to *pb_extension_t*. +PB_LTYPE_FIXED_LENGTH_BYTES 0x09 Inline *pb_byte_t* array of fixed size. +=========================== ===== ================================================ The bits 4-5 define whether the field is required, optional or repeated: @@ -489,14 +494,14 @@ This function only considers the LTYPE of the field. You can use it from your fi Wire type mapping is as follows: -========================= ============ -LTYPEs Wire type -========================= ============ -VARINT, SVARINT PB_WT_VARINT -FIXED64 PB_WT_64BIT -STRING, BYTES, SUBMESSAGE PB_WT_STRING -FIXED32 PB_WT_32BIT -========================= ============ +============================================= ============ +LTYPEs Wire type +============================================= ============ +VARINT, UVARINT, SVARINT PB_WT_VARINT +FIXED64 PB_WT_64BIT +STRING, BYTES, SUBMESSAGE, FIXED_LENGTH_BYTES PB_WT_STRING +FIXED32 PB_WT_32BIT +============================================= ============ pb_encode_varint ---------------- diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index 9cf2de5f..973c7610 100755 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -241,6 +241,11 @@ class Field: self.enc_size = None self.ctype = None + self.inline = None + if field_options.type == nanopb_pb2.FT_INLINE: + field_options.type = nanopb_pb2.FT_STATIC + self.inline = nanopb_pb2.FT_INLINE + # Parse field options if field_options.HasField("max_size"): self.max_size = field_options.max_size @@ -319,7 +324,12 @@ class Field: elif desc.type == FieldD.TYPE_BYTES: self.pbtype = 'BYTES' if self.allocation == 'STATIC': - self.ctype = self.struct_name + self.name + 't' + # Inline STATIC for BYTES is like STATIC for STRING. + if self.inline: + self.ctype = 'pb_byte_t' + self.array_decl += '[%d]' % self.max_size + else: + 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_array_t' @@ -359,7 +369,7 @@ class Field: def types(self): '''Return definitions for any special types this field might need.''' - if self.pbtype == 'BYTES' and self.allocation == 'STATIC': + if self.pbtype == 'BYTES' and self.allocation == 'STATIC' and not self.inline: result = 'typedef PB_BYTES_ARRAY_T(%d) %s;\n' % (self.max_size, self.ctype) else: result = '' @@ -388,7 +398,10 @@ class Field: if self.pbtype == 'STRING': inner_init = '""' elif self.pbtype == 'BYTES': - inner_init = '{0, {0}}' + if self.inline: + inner_init = '{0}' + else: + inner_init = '{0, {0}}' elif self.pbtype in ('ENUM', 'UENUM'): inner_init = '(%s)0' % self.ctype else: @@ -400,9 +413,15 @@ class Field: elif self.pbtype == 'BYTES': data = ['0x%02x' % ord(c) for c in self.default] if len(data) == 0: - inner_init = '{0, {0}}' + if self.inline: + inner_init = '{0}' + else: + inner_init = '{0, {0}}' else: - inner_init = '{%d, {%s}}' % (len(data), ','.join(data)) + if self.inline: + inner_init = '{%s}' % ','.join(data) + else: + inner_init = '{%d, {%s}}' % (len(data), ','.join(data)) elif self.pbtype in ['FIXED32', 'UINT32']: inner_init = str(self.default) + 'u' elif self.pbtype in ['FIXED64', 'UINT64']: @@ -454,6 +473,8 @@ class Field: elif self.pbtype == 'BYTES': if self.allocation != 'STATIC': return None # Not implemented + if self.inline: + array_decl = '[%d]' % self.max_size if declaration_only: return 'extern const %s %s_default%s;' % (ctype, self.struct_name + self.name, array_decl) @@ -481,7 +502,7 @@ class Field: result += '%3d, ' % self.tag result += '%-8s, ' % self.pbtype result += '%s, ' % self.rules - result += '%-8s, ' % self.allocation + result += '%-8s, ' % (self.allocation if not self.inline else "INLINE") result += '%s, ' % ("FIRST" if not prev_field_name else "OTHER") result += '%s, ' % self.struct_name result += '%s, ' % self.name @@ -594,6 +615,7 @@ class ExtensionRange(Field): self.default = None self.max_size = 0 self.max_count = 0 + self.inline = None def __str__(self): return ' pb_extension_t *extensions;' @@ -671,6 +693,7 @@ class OneOf(Field): self.default = None self.rules = 'ONEOF' self.anonymous = False + self.inline = None def add_field(self, field): if field.allocation == 'CALLBACK': diff --git a/generator/proto/nanopb.proto b/generator/proto/nanopb.proto index 9b2f0fbc..8aab19a1 100644 --- a/generator/proto/nanopb.proto +++ b/generator/proto/nanopb.proto @@ -16,6 +16,7 @@ enum FieldType { FT_POINTER = 4; // Always generate a dynamically allocated field. FT_STATIC = 2; // Generate a static field or raise an exception if not possible. FT_IGNORE = 3; // Ignore the field completely. + FT_INLINE = 5; // Always generate an inline array of fixed size. } enum IntSize { diff --git a/pb.h b/pb.h index 790f8861..4576f79a 100644 --- a/pb.h +++ b/pb.h @@ -170,8 +170,14 @@ typedef uint_least8_t pb_type_t; * The field contains a pointer to pb_extension_t */ #define PB_LTYPE_EXTENSION 0x08 +/* Byte array with inline, pre-allocated byffer. + * data_size is the length of the inline, allocated buffer. + * This differs from PB_LTYPE_BYTES by defining the element as + * pb_byte_t[data_size] rather than pb_bytes_array_t. */ +#define PB_LTYPE_FIXED_LENGTH_BYTES 0x09 + /* Number of declared LTYPES */ -#define PB_LTYPES_COUNT 9 +#define PB_LTYPES_COUNT 0x0A #define PB_LTYPE_MASK 0x0F /**** Field repetition rules ****/ @@ -415,6 +421,19 @@ struct pb_extension_s { pb_membersize(st, m[0]), \ pb_arraysize(st, m), ptr} +#define PB_REQUIRED_INLINE(tag, st, m, fd, ltype, ptr) \ + {tag, PB_ATYPE_STATIC | PB_HTYPE_REQUIRED | PB_LTYPE_FIXED_LENGTH_BYTES, \ + fd, 0, pb_membersize(st, m), 0, ptr} + +/* Optional fields add the delta to the has_ variable. */ +#define PB_OPTIONAL_INLINE(tag, st, m, fd, ltype, ptr) \ + {tag, PB_ATYPE_STATIC | PB_HTYPE_OPTIONAL | PB_LTYPE_FIXED_LENGTH_BYTES, \ + fd, \ + pb_delta(st, has_ ## m, m), \ + pb_membersize(st, m), 0, ptr} + +/* INLINE does not support REPEATED fields. */ + /* Allocated fields carry the size of the actual data, not the pointer */ #define PB_REQUIRED_POINTER(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_POINTER | PB_HTYPE_REQUIRED | ltype, \ @@ -454,6 +473,8 @@ struct pb_extension_s { #define PB_OPTEXT_POINTER(tag, st, m, fd, ltype, ptr) \ PB_OPTIONAL_POINTER(tag, st, m, fd, ltype, ptr) +/* INLINE does not support OPTEXT. */ + #define PB_OPTEXT_CALLBACK(tag, st, m, fd, ltype, ptr) \ PB_OPTIONAL_CALLBACK(tag, st, m, fd, ltype, ptr) @@ -485,7 +506,7 @@ struct pb_extension_s { * FLOAT, INT32, INT64, MESSAGE, SFIXED32, SFIXED64 * SINT32, SINT64, STRING, UINT32, UINT64 or EXTENSION * - Field rules: REQUIRED, OPTIONAL or REPEATED - * - Allocation: STATIC or CALLBACK + * - Allocation: STATIC, INLINE, or CALLBACK * - Placement: FIRST or OTHER, depending on if this is the first field in structure. * - Message name * - Field name @@ -511,6 +532,8 @@ struct pb_extension_s { fd, pb_delta(st, which_ ## u, u.m), \ pb_membersize(st, u.m[0]), 0, ptr} +/* INLINE does not support ONEOF. */ + #define PB_ONEOF_FIELD(union_name, tag, type, rules, allocation, placement, message, field, prevfield, ptr) \ PB_ONEOF_ ## allocation(union_name, tag, message, field, \ PB_DATAOFFSET_ ## placement(message, union_name.field, prevfield), \ diff --git a/pb_decode.c b/pb_decode.c index 78911e7b..7a4e29a8 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -65,7 +65,8 @@ static const pb_decoder_t PB_DECODERS[PB_LTYPES_COUNT] = { &pb_dec_bytes, &pb_dec_string, &pb_dec_submessage, - NULL /* extensions */ + NULL, /* extensions */ + &pb_dec_bytes /* PB_LTYPE_FIXED_LENGTH_BYTES */ }; /******************************* @@ -1272,6 +1273,12 @@ static bool checkreturn pb_dec_bytes(pb_istream_t *stream, const pb_field_t *fie } else { + if (PB_LTYPE(field->type) == PB_LTYPE_FIXED_LENGTH_BYTES) { + if (size != field->data_size) + PB_RETURN_ERROR(stream, "incorrect inline bytes size"); + return pb_read(stream, (pb_byte_t*)dest, field->data_size); + } + if (alloc_size > field->data_size) PB_RETURN_ERROR(stream, "bytes overflow"); bdest = (pb_bytes_array_t*)dest; diff --git a/pb_encode.c b/pb_encode.c index 9f91c9d5..4685614c 100644 --- a/pb_encode.c +++ b/pb_encode.c @@ -49,7 +49,8 @@ static const pb_encoder_t PB_ENCODERS[PB_LTYPES_COUNT] = { &pb_enc_bytes, &pb_enc_string, &pb_enc_submessage, - NULL /* extensions */ + NULL, /* extensions */ + &pb_enc_bytes /* PB_LTYPE_FIXED_LENGTH_BYTES */ }; /******************************* @@ -498,6 +499,7 @@ bool checkreturn pb_encode_tag_for_field(pb_ostream_t *stream, const pb_field_t case PB_LTYPE_BYTES: case PB_LTYPE_STRING: case PB_LTYPE_SUBMESSAGE: + case PB_LTYPE_FIXED_LENGTH_BYTES: wiretype = PB_WT_STRING; break; @@ -636,11 +638,16 @@ static bool checkreturn pb_enc_fixed32(pb_ostream_t *stream, const pb_field_t *f static bool checkreturn pb_enc_bytes(pb_ostream_t *stream, const pb_field_t *field, const void *src) { - const pb_bytes_array_t *bytes = (const pb_bytes_array_t*)src; + const pb_bytes_array_t *bytes = NULL; + + if (PB_LTYPE(field->type) == PB_LTYPE_FIXED_LENGTH_BYTES) + return pb_encode_string(stream, (const pb_byte_t*)src, field->data_size); + + bytes = (const pb_bytes_array_t*)src; if (src == NULL) { - /* Threat null pointer as an empty bytes field */ + /* Treat null pointer as an empty bytes field */ return pb_encode_string(stream, NULL, 0); } @@ -664,7 +671,7 @@ static bool checkreturn pb_enc_string(pb_ostream_t *stream, const pb_field_t *fi if (src == NULL) { - size = 0; /* Threat null pointer as an empty string */ + size = 0; /* Treat null pointer as an empty string */ } else { diff --git a/tests/inline/SConscript b/tests/inline/SConscript new file mode 100644 index 00000000..34371fda --- /dev/null +++ b/tests/inline/SConscript @@ -0,0 +1,16 @@ +# Test that inlined bytes fields work. + +Import("env") + +env.NanopbProto("inline") +env.Object("inline.pb.c") + +env.Match(["inline.pb.h", "inline.expected"]) + +p = env.Program(["inline_unittests.c", + "inline.pb.c", + "$COMMON/pb_encode.o", + "$COMMON/pb_decode.o", + "$COMMON/pb_common.o"]) + +env.RunTest(p) diff --git a/tests/inline/inline.expected b/tests/inline/inline.expected new file mode 100644 index 00000000..593e972b --- /dev/null +++ b/tests/inline/inline.expected @@ -0,0 +1,3 @@ +pb_byte_t data\[32\]; +bool has_data; +pb_byte_t data\[64\]; diff --git a/tests/inline/inline.proto b/tests/inline/inline.proto new file mode 100644 index 00000000..6e511f0a --- /dev/null +++ b/tests/inline/inline.proto @@ -0,0 +1,17 @@ +/* Test nanopb option parsing. + * options.expected lists the patterns that are searched for in the output. + */ + +syntax = "proto2"; + +import "nanopb.proto"; + +message Message1 +{ + required bytes data = 1 [(nanopb).type = FT_INLINE, (nanopb).max_size = 32]; +} + +message Message2 +{ + optional bytes data = 1 [(nanopb).type = FT_INLINE, (nanopb).max_size = 64]; +} diff --git a/tests/inline/inline_unittests.c b/tests/inline/inline_unittests.c new file mode 100644 index 00000000..b5834c7e --- /dev/null +++ b/tests/inline/inline_unittests.c @@ -0,0 +1,73 @@ +#include +#include +#include +#include +#include "unittests.h" +#include "inline.pb.h" + +int main() +{ + int status = 0; + int i = 0; + COMMENT("Test inline byte fields"); + + { + Message1 msg1 = Message1_init_zero; + TEST(sizeof(msg1.data) == 32); + } + + { + Message1 msg1 = Message1_init_zero; + pb_byte_t msg1_buffer[Message1_size]; + pb_ostream_t ostream = pb_ostream_from_buffer(msg1_buffer, Message1_size); + Message1 msg1_deserialized = Message1_init_zero; + pb_istream_t istream = pb_istream_from_buffer(msg1_buffer, Message1_size); + + for (i = 0; i < 32; i++) { + msg1.data[i] = i; + } + + TEST(pb_encode(&ostream, Message1_fields, &msg1)); + TEST(ostream.bytes_written == Message1_size); + + TEST(pb_decode(&istream, Message1_fields, &msg1_deserialized)); + + TEST(istream.bytes_left == 0); + TEST(memcmp(&msg1_deserialized, &msg1, sizeof(msg1)) == 0); + } + + { + Message2 msg2 = {true, {0}}; + Message2 msg2_no_data = {false, {1}}; + pb_byte_t msg2_buffer[Message2_size]; + pb_ostream_t ostream = pb_ostream_from_buffer(msg2_buffer, Message2_size); + Message2 msg2_deserialized = Message2_init_zero; + pb_istream_t istream = pb_istream_from_buffer(msg2_buffer, Message2_size); + + for (i = 0; i < 64; i++) { + msg2.data[i] = i; + } + + TEST(pb_encode(&ostream, Message2_fields, &msg2)); + TEST(ostream.bytes_written == Message2_size); + + TEST(pb_decode(&istream, Message2_fields, &msg2_deserialized)); + + TEST(istream.bytes_left == 0); + TEST(memcmp(&msg2_deserialized, &msg2, sizeof(msg2)) == 0); + TEST(msg2_deserialized.has_data); + + memset(msg2_buffer, 0, sizeof(msg2_buffer)); + ostream = pb_ostream_from_buffer(msg2_buffer, Message2_size); + TEST(pb_encode(&ostream, Message2_fields, &msg2_no_data)); + istream = pb_istream_from_buffer(msg2_buffer, Message2_size); + TEST(pb_decode(&istream, Message2_fields, &msg2_deserialized)); + TEST(!msg2_deserialized.has_data); + TEST(memcmp(&msg2_deserialized, &msg2, sizeof(msg2)) != 0); + } + + if (status != 0) + fprintf(stdout, "\n\nSome tests FAILED!\n"); + + return status; +} -- 2.16.6