From: Petteri Aimonen <jpa@git.mail.kapsi.fi>
Date: Sun, 13 Sep 2015 08:38:54 +0000 (+0300)
Subject: Fix maximum encoded size for negative enums (issue #166).
X-Git-Tag: 5.0.2~186^2~118
X-Git-Url: https://gerrit.automotivelinux.org/gerrit/gitweb?a=commitdiff_plain;h=6e72df4808aa138f1396ad098ce2d06a6feba882;p=apps%2Fagl-service-can-low-level.git

Fix maximum encoded size for negative enums (issue #166).
---

diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index f9d98484..3a5fac5a 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -100,11 +100,14 @@ def names_from_type_name(type_name):
 
 def varint_max_size(max_value):
     '''Returns the maximum number of bytes a varint can take when encoded.'''
+    if max_value < 0:
+        max_value = 2**64 - max_value
     for i in range(1, 11):
         if (max_value >> (i * 7)) == 0:
             return i
     raise ValueError("Value too large for varint: " + str(max_value))
 
+assert varint_max_size(-1) == 10
 assert varint_max_size(0) == 1
 assert varint_max_size(127) == 1
 assert varint_max_size(128) == 2
@@ -168,6 +171,9 @@ class Enum:
                 return True
         return False
     
+    def encoded_size(self):
+        return max([varint_max_size(v) for n,v in self.values])
+    
     def __str__(self):
         result = 'typedef enum _%s {\n' % self.names
         result += ',\n'.join(["    %s = %d" % x for x in self.values])
@@ -267,7 +273,7 @@ class Field:
             self.ctype = names_from_type_name(desc.type_name)
             if self.default is not None:
                 self.default = self.ctype + self.default
-            self.enc_size = 5 # protoc rejects enum values > 32 bits
+            self.enc_size = None # Needs to be filled in when enum values are known
         elif desc.type == FieldD.TYPE_STRING:
             self.pbtype = 'STRING'
             self.ctype = 'char'
@@ -500,6 +506,14 @@ class Field:
                 # prefix size, though.
                 encsize += 5
 
+        elif self.pbtype in ['ENUM', 'UENUM']:
+            if str(self.ctype) in dependencies:
+                enumtype = dependencies[str(self.ctype)]
+                encsize = enumtype.encoded_size()
+            else:
+                # Conservative assumption
+                encsize = 10
+
         elif self.enc_size is None:
             raise RuntimeError("Could not determine encoded size for %s.%s"
                                % (self.struct_name, self.name))
diff --git a/tests/regression/issue_166/SConscript b/tests/regression/issue_166/SConscript
new file mode 100644
index 00000000..c50b9193
--- /dev/null
+++ b/tests/regression/issue_166/SConscript
@@ -0,0 +1,13 @@
+# Verify that the maximum encoded size is calculated properly
+# for enums.
+
+Import('env')
+
+env.NanopbProto('enums')
+
+p = env.Program(["enum_encoded_size.c",
+                 "enums.pb.c",
+                 "$COMMON/pb_encode.o",
+                 "$COMMON/pb_common.o"])
+env.RunTest(p)
+
diff --git a/tests/regression/issue_166/enum_encoded_size.c b/tests/regression/issue_166/enum_encoded_size.c
new file mode 100644
index 00000000..84e1c7de
--- /dev/null
+++ b/tests/regression/issue_166/enum_encoded_size.c
@@ -0,0 +1,43 @@
+#include <stdio.h>
+#include <string.h>
+#include <pb_encode.h>
+#include "unittests.h"
+#include "enums.pb.h"
+
+int main()
+{
+    int status = 0;
+    
+    uint8_t buf[256];
+    SignedMsg msg1;
+    UnsignedMsg msg2;
+    pb_ostream_t s;
+    
+    {
+        COMMENT("Test negative value of signed enum");
+        /* Negative value should take up the maximum size */
+        msg1.value = SignedEnum_SE_MIN;
+        s = pb_ostream_from_buffer(buf, sizeof(buf));
+        TEST(pb_encode(&s, SignedMsg_fields, &msg1));
+        TEST(s.bytes_written == SignedMsg_size);
+        
+        COMMENT("Test positive value of signed enum");
+        /* Positive value should be smaller */
+        msg1.value = SignedEnum_SE_MAX;
+        s = pb_ostream_from_buffer(buf, sizeof(buf));
+        TEST(pb_encode(&s, SignedMsg_fields, &msg1));
+        TEST(s.bytes_written < SignedMsg_size);
+    }
+    
+    {
+        COMMENT("Test positive value of unsigned enum");
+        /* This should take up the maximum size */
+        msg2.value = UnsignedEnum_UE_MAX;
+        s = pb_ostream_from_buffer(buf, sizeof(buf));
+        TEST(pb_encode(&s, UnsignedMsg_fields, &msg2));
+        TEST(s.bytes_written == UnsignedMsg_size);
+    }
+    
+    return status;
+}
+
diff --git a/tests/regression/issue_166/enums.proto b/tests/regression/issue_166/enums.proto
new file mode 100644
index 00000000..a0964ab4
--- /dev/null
+++ b/tests/regression/issue_166/enums.proto
@@ -0,0 +1,16 @@
+enum SignedEnum {
+    SE_MIN = -1;
+    SE_MAX = 255;
+}
+
+enum UnsignedEnum {
+    UE_MAX = 65536;
+}
+
+message SignedMsg {
+    required SignedEnum value = 1;
+}
+
+message UnsignedMsg {
+    required UnsignedEnum value = 1;
+}