Merge pull request #211 from tmroeder/feat-inline-bytes

Add inline allocation of bytes fields

13 files changed, 239 insertions, 38 deletions

diff --git a/AUTHORS b/AUTHORS
new file mode 100644
index 00000000..a967d3ec
--- /dev/null
+++ b/AUTHORS
@@ -0,0 +1,24 @@
+Petteri Aimonen <jpa@npb.mail.kapsi.fi>
+Michael Poole <mdpoole@troilus.org>
+Daniel Kan <extremeblue99@gmail.com>
+Stan Hu <stanhu@aclimalabs.com>
+dch <david.hotham@blueyonder.co.uk>
+Steffen Siering <steffen siering gmail com>
+Jens Steinhauser <jens.steinhauser@gmail.com>
+Pavel Ilin <ilin.pa@gmail.com>
+Kent Ryhorchuk <kryhorchuk@xeralux.com>
+Martin Donath <scifish@gmail.com>
+Oliver Lee <oliverzlee@gmail.com>
+Michael Haberler <git@mah.priv.at>
+Nicolas Colomer <ncolomer@viadeoteam.com>
+Ivan Kravets <me@ikravets.com>
+Kyle Manna <kyle@kylemanna.com>
+Benjamin Kamath <ben.kamath@synapse.com>
+Andrew Ruder <andrew.ruder@elecsyscorp.com>
+Kenshi Kawaguchi <kenshi@recurse.ca>
+isotes <isotes@gmail.com>
+Maxim Khitrov <max@mxcrypt.com>
+Yaniv Mordekhay <yanivmo@users.noreply.github.com>
+Ming Zhao <mzhao@luminatewireless.com>
+Google, Inc.
+Tom Roeder <tmroeder@google.com>
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 <stdio.h>
+#include <string.h>
+#include <pb_decode.h>
+#include <pb_encode.h>
+#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;
+}