Fix unordered field numbers inside oneof causing fields to be ignored (#617)
Previous fix in commit ee1aff9 was incorrect.
The descriptor array should be kept ordered and generator is now fixed
to do so.
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index 578deb7..b95fb41 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -1068,9 +1068,6 @@
def tags(self):
return ''.join([f.tags() for f in self.fields])
- def fieldlist(self):
- return ' \\\n'.join(field.fieldlist() for field in self.fields)
-
def data_size(self, dependencies):
return max(f.data_size(dependencies) for f in self.fields)
@@ -1305,10 +1302,14 @@
while any(field.name == Field.macro_a_param for field in self.all_fields()):
Field.macro_a_param += '_'
+ # Field descriptor array must be sorted by tag number, pb_common.c relies on it.
+ sorted_fields = list(self.all_fields())
+ sorted_fields.sort(key = lambda x: x.tag)
+
result = '#define %s_FIELDLIST(%s, %s) \\\n' % (self.name,
Field.macro_x_param,
Field.macro_a_param)
- result += ' \\\n'.join(field.fieldlist() for field in sorted(self.fields))
+ result += ' \\\n'.join(x.fieldlist() for x in sorted_fields)
result += '\n'
has_callbacks = bool([f for f in self.fields if f.has_callbacks()])
@@ -1326,13 +1327,12 @@
else:
result += '#define %s_DEFAULT NULL\n' % self.name
- for field in sorted(self.fields):
+ for field in sorted_fields:
if field.pbtype in ['MESSAGE', 'MSG_W_CB']:
- result += "#define %s_%s_MSGTYPE %s\n" % (self.name, field.name, field.ctype)
- elif field.rules == 'ONEOF':
- for member in field.fields:
- if member.pbtype in ['MESSAGE', 'MSG_W_CB']:
- result += "#define %s_%s_%s_MSGTYPE %s\n" % (self.name, member.union_name, member.name, member.ctype)
+ if field.rules == 'ONEOF':
+ result += "#define %s_%s_%s_MSGTYPE %s\n" % (self.name, field.union_name, field.name, field.ctype)
+ else:
+ result += "#define %s_%s_MSGTYPE %s\n" % (self.name, field.name, field.ctype)
return result
diff --git a/tests/common_unittests/common_unittests.c b/tests/common_unittests/common_unittests.c
index b6ed46d..8452b35 100644
--- a/tests/common_unittests/common_unittests.c
+++ b/tests/common_unittests/common_unittests.c
@@ -88,12 +88,11 @@
TEST(pb_field_iter_next(&iter) && iter.tag == 60 && iter.pData == &msg.oneof.oneof_msg1 && iter.pSize == &msg.which_oneof )
TEST(pb_field_iter_next(&iter) && iter.tag == 61 && iter.pData == &msg.oneof.oneof_msg2 && iter.pSize == &msg.which_oneof )
+ TEST(pb_field_iter_next(&iter) && iter.tag == 62 && iter.pData == &msg.opt_non_zero_based_enum && iter.pSize == &msg.has_opt_non_zero_based_enum)
TEST(pb_field_iter_next(&iter) && iter.tag == 63 && iter.pData == &msg.oneof.static_msg && iter.pSize == &msg.which_oneof )
TEST(iter.required_field_index == 19)
TEST(iter.submessage_index == 8)
- TEST(pb_field_iter_next(&iter) && iter.tag == 62 && iter.pData == &msg.opt_non_zero_based_enum && iter.pSize == &msg.has_opt_non_zero_based_enum)
-
TEST(pb_field_iter_next(&iter) && iter.tag == 95 && iter.pData == &msg.rep_farray2 && iter.pSize == &iter.array_size && iter.array_size == 3)
TEST(iter.required_field_index == 19)
TEST(iter.submessage_index == 9)
diff --git a/tests/regression/issue_617/SConscript b/tests/regression/issue_617/SConscript
new file mode 100644
index 0000000..80ca106
--- /dev/null
+++ b/tests/regression/issue_617/SConscript
@@ -0,0 +1,10 @@
+# Regression test for #617:
+# Unordered field numbers inside oneof can cause fields to be ignored
+
+Import("env")
+
+env.NanopbProto(["oneof.proto", "oneof.options"])
+
+test = env.Program(["test_oneof.c", "oneof.pb.c", "$COMMON/pb_decode.o", "$COMMON/pb_common.o"])
+env.RunTest(test)
+
diff --git a/tests/regression/issue_617/oneof.options b/tests/regression/issue_617/oneof.options
new file mode 100644
index 0000000..cdbbd51
--- /dev/null
+++ b/tests/regression/issue_617/oneof.options
@@ -0,0 +1,4 @@
+* long_names : false
+
+NoDecode.name max_length : 32
+
diff --git a/tests/regression/issue_617/oneof.proto b/tests/regression/issue_617/oneof.proto
new file mode 100644
index 0000000..914d5f3
--- /dev/null
+++ b/tests/regression/issue_617/oneof.proto
@@ -0,0 +1,41 @@
+syntax = "proto3";
+
+message DecodesOK
+{
+ uint32 a = 1;
+ uint32 b = 2;
+}
+
+message NoDecode
+{
+ string name = 1;
+}
+
+message TestMessage
+{
+ enum MessageType
+ {
+ A = 0;
+ B = 1;
+ C = 2;
+ }
+
+ MessageType messageType = 1;
+ uint32 x = 2;
+ uint32 y = 3;
+ uint32 ip1 = 4;
+
+ oneof payload
+ {
+ DecodesOK pla5 = 5;
+ DecodesOK pla6 = 6;
+ DecodesOK pla7 = 7;
+ DecodesOK pla8 = 8;
+ DecodesOK pla9 = 9;
+ DecodesOK pla10 = 10;
+ NoDecode plb11 = 12;
+ }
+
+ uint32 ip2 = 11;
+}
+
diff --git a/tests/regression/issue_617/test_oneof.c b/tests/regression/issue_617/test_oneof.c
new file mode 100644
index 0000000..7a89fad
--- /dev/null
+++ b/tests/regression/issue_617/test_oneof.c
@@ -0,0 +1,22 @@
+#include <pb_decode.h>
+#include <unittests.h>
+#include "oneof.pb.h"
+
+int main()
+{
+ const uint8_t input_data[] = {
+ 0x08, 0x01, 0x10, 0x0F, 0x18, 0xAC, 0x02, 0x20,
+ 0xF1, 0x82, 0xA0, 0x85, 0x0C, 0x62, 0x00, 0x58,
+ 0xF1, 0x82, 0xA0, 0x85, 0x0C
+ };
+
+ int status = 0;
+ TestMessage msg = TestMessage_init_zero;
+ pb_istream_t stream = pb_istream_from_buffer(input_data, sizeof(input_data));
+
+ TEST(pb_decode(&stream, TestMessage_fields, &msg));
+ TEST(msg.which_payload == TestMessage_plb11_tag);
+ TEST(msg.payload.plb11.name[0] == 0);
+
+ return status;
+}
