random todo items

git-svn-id: https://protobuf-c.googlecode.com/svn/trunk@35 00440858-1255-0410-a3e6-75ea37f81c3a

TODO

@@ -2,13 +2,14 @@
 - handle unknown fields when packing
 - test code
   - ensure enums are 32-bit
-- make messages derive from ProtobufCMessage,
-  which should have the unknown field array
 - make services derive from ProtobufCService;
   be usable with a cast.
 - provide example rpc
-- use size_t wherever appropriate in generated code, and in uses of that code (eg
-									       pack and unpack)
+- use size_t wherever appropriate in generated code,
+  and in uses of that code (eg pack and unpack)
+
+- To document:
+  - __INIT macro
 
 - support Group (whatever it is)
 - almost no code generator options are obeyed

src/google/protobuf-c/protobuf-c-private.h

@@ -31,3 +31,6 @@ struct _ProtobufCIntRange
 int protobuf_c_int_ranges_lookup (unsigned n_ranges,
                                   ProtobufCIntRange *ranges);
 
+#define PROTOBUF_C_SERVICE_DESCRIPTOR_MAGIC  0x14159bc3
+#define PROTOBUF_C_MESSAGE_DESCRIPTOR_MAGIC  0x28aaeef9
+#define PROTOBUF_C_ENUM_DESCRIPTOR_MAGIC     0x114315af

src/google/protobuf-c/protobuf-c.c

@@ -823,7 +823,7 @@ parse_tag_and_wiretype (size_t len,
   for (rv = 1; rv < max_rv; rv++)
     if (data[rv] & 0x80)
       {
-        tag |= (data[rv++] & 0x7f) << shift;
+        tag |= (data[rv] & 0x7f) << shift;
         shift += 7;
       }
     else
@@ -1110,8 +1110,7 @@ parse_member (ScannedMember *scanned_member,
   void *member;
   if (field == NULL)
     {
-      ProtobufCMessageUnknownFieldArray *array = MESSAGE_GET_UNKNOWNS (message);
-      ProtobufCMessageUnknownField *ufield = array->unknown_fields + (array->n_unknown_fields++);
+      ProtobufCMessageUnknownField *ufield = message->unknown_fields + (message->n_unknown_fields++);
       ufield->tag = scanned_member->tag;
       ufield->wire_type = scanned_member->wire_type;
       ufield->len = scanned_member->len;
@@ -1157,7 +1156,7 @@ protobuf_c_message_unpack         (const ProtobufCMessageDescriptor *desc,
   rv = ALLOC (allocator, desc->sizeof_message);
   scanned_member_slabs[0] = first_member_slab;
 
-  memset (rv + 1, 0, desc->sizeof_message - sizeof (ProtobufCMessage));
+  memset (rv, 0, desc->sizeof_message);
   rv->descriptor = desc;
 
   while (rem > 0)
@@ -1274,8 +1273,7 @@ protobuf_c_message_unpack         (const ProtobufCMessageDescriptor *desc,
   /* allocate space for unknown fields */
   if (n_unknown)
     {
-      ProtobufCMessageUnknownFieldArray *array = MESSAGE_GET_UNKNOWNS (rv);
-      array->unknown_fields = ALLOC (allocator, n_unknown * sizeof (ProtobufCMessageUnknownField));
+      rv->unknown_fields = ALLOC (allocator, n_unknown * sizeof (ProtobufCMessageUnknownField));
     }
 
   /* do real parsing */

src/google/protobuf-c/protobuf-c.h

@@ -86,6 +86,8 @@ struct _ProtobufCEnumValue
 
 struct _ProtobufCEnumDescriptor
 {
+  uint32_t magic;
+
   const char *name;
   const char *short_name;
   const char *c_name;
@@ -115,6 +117,8 @@ struct _ProtobufCFieldDescriptor
 };
 struct _ProtobufCMessageDescriptor
 {
+  uint32_t magic;
+
   const char *name;
   const char *short_name;
   const char *c_name;
@@ -129,16 +133,17 @@ struct _ProtobufCMessageDescriptor
   /* ranges, optimization for looking up fields */
   unsigned n_field_ranges;
   const ProtobufCIntRange *field_ranges;
-
-  /* offset in message to the array of unknown fields */
-  unsigned unknown_field_array_offset;
 };
 
 typedef struct _ProtobufCMessage ProtobufCMessage;
+typedef struct _ProtobufCMessageUnknownField ProtobufCMessageUnknownField;
 struct _ProtobufCMessage
 {
   const ProtobufCMessageDescriptor *descriptor;
+  unsigned n_unknown_fields;
+  ProtobufCMessageUnknownField *unknown_fields;
 };
+#define PROTOBUF_C_MESSAGE_INIT(descriptor) { descriptor, 0, NULL }
 
 size_t    protobuf_c_message_get_packed_size(const ProtobufCMessage *message);
 size_t    protobuf_c_message_pack           (const ProtobufCMessage *message,
@@ -167,6 +172,8 @@ struct _ProtobufCMethodDescriptor
 };
 struct _ProtobufCServiceDescriptor
 {
+  uint32_t magic;
+
   const char *name;
   const char *short_name;
   const char *c_name;
@@ -207,8 +214,6 @@ typedef enum
 } ProtobufCWireType;
 
 /* --- unknown message fields --- */
-typedef struct _ProtobufCMessageUnknownField ProtobufCMessageUnknownField;
-typedef struct _ProtobufCMessageUnknownFieldArray ProtobufCMessageUnknownFieldArray;
 struct _ProtobufCMessageUnknownField
 {
   uint32_t tag;
@@ -216,11 +221,6 @@ struct _ProtobufCMessageUnknownField
   size_t len;
   unsigned char *data;
 };
-struct _ProtobufCMessageUnknownFieldArray
-{
-  unsigned n_unknown_fields;
-  ProtobufCMessageUnknownField *unknown_fields;
-};
 
 /* --- extra (superfluous) api:  trivial buffer --- */
 typedef struct _ProtobufCBufferSimple ProtobufCBufferSimple;

src/google/protobuf/compiler/c/c_enum.cc

@@ -185,6 +185,7 @@ void EnumGenerator::GenerateEnumDescriptor(io::Printer* printer) {
   printer->Print(vars,
     "const ProtobufCEnumDescriptor $lcclassname$__descriptor =\n"
     "{\n"
+    "  PROTOBUF_C_ENUM_DESCRIPTOR_MAGIC,\n"
     "  \"$fullname$\",\n"
     "  \"$shortname$\",\n"
     "  \"$cname$\",\n"

src/google/protobuf/compiler/c/c_message.cc

@@ -110,7 +110,7 @@ GenerateStructDefinition(io::Printer* printer) {
   printer->Print(vars,
     "struct $dllexport$ _$classname$\n"
     "{\n"
-    "  const ProtobufCMessageDescriptor *descriptor;\n");
+    "  ProtobufCMessage base;\n");
 
   // Generate fields.
   printer->Indent();
@@ -120,12 +120,10 @@ GenerateStructDefinition(io::Printer* printer) {
   }
   printer->Outdent();
 
-  printer->Print(vars, "  /* private */\n");
-  printer->Print(vars, "  ProtobufCMessageUnknownFieldArray unknown_field_array;\n");
   printer->Print(vars, "};\n\n");
 
   printer->Print(vars, "#define $ucclassname$__INIT \\\n"
-		       " { &$lcclassname$__descriptor");
+		       " { PROTOBUF_C_MESSAGE_INIT (&$lcclassname$__descriptor)");
   for (int i = 0; i < descriptor_->field_count(); i++) {
     const FieldDescriptor *field = descriptor_->field(i);
     printer->Print(", ");
@@ -208,21 +206,21 @@ GenerateHelperFunctionDefinitions(io::Printer* printer)
 		 "size_t $lcclassname$__get_packed_size\n"
 		 "                     (const $classname$ *message)\n"
 		 "{\n"
-		 "  PROTOBUF_C_ASSERT (message->descriptor == &$lcclassname$__descriptor);\n"
+		 "  PROTOBUF_C_ASSERT (message->base.descriptor == &$lcclassname$__descriptor);\n"
 		 "  return protobuf_c_message_get_packed_size ((ProtobufCMessage*)(message));\n"
 		 "}\n"
 		 "size_t $lcclassname$__pack\n"
 		 "                     (const $classname$ *message,\n"
 		 "                      unsigned char *out)\n"
 		 "{\n"
-		 "  PROTOBUF_C_ASSERT (message->descriptor == &$lcclassname$__descriptor);\n"
+		 "  PROTOBUF_C_ASSERT (message->base.descriptor == &$lcclassname$__descriptor);\n"
 		 "  return protobuf_c_message_pack ((ProtobufCMessage*)message, out);\n"
 		 "}\n"
 		 "size_t $lcclassname$__pack_to_buffer\n"
 		 "                     (const $classname$ *message,\n"
 		 "                      ProtobufCBuffer *buffer)\n"
 		 "{\n"
-		 "  PROTOBUF_C_ASSERT (message->descriptor == &$lcclassname$__descriptor);\n"
+		 "  PROTOBUF_C_ASSERT (message->base.descriptor == &$lcclassname$__descriptor);\n"
 		 "  return protobuf_c_message_pack_to_buffer ((ProtobufCMessage*)message, buffer);\n"
 		 "}\n"
 		 "$classname$ *\n"
@@ -239,7 +237,7 @@ GenerateHelperFunctionDefinitions(io::Printer* printer)
 		 "                     ($classname$ *message,\n"
 		 "                      ProtobufCAllocator *allocator)\n"
 		 "{\n"
-		 "  PROTOBUF_C_ASSERT (message->descriptor == &$lcclassname$__descriptor);\n"
+		 "  PROTOBUF_C_ASSERT (message->base.descriptor == &$lcclassname$__descriptor);\n"
 		 "  protobuf_c_message_free_unpacked ((ProtobufCMessage*)message, allocator);\n"
 		 "}\n"
 		);
@@ -297,6 +295,7 @@ GenerateMessageDescriptor(io::Printer* printer) {
   printer->Print(vars,
     "const ProtobufCMessageDescriptor $lcclassname$__descriptor =\n"
     "{\n"
+    "  PROTOBUF_C_MESSAGE_DESCRIPTOR_MAGIC,\n"
     "  \"$fullname$\",\n"
     "  \"$shortname$\",\n"
     "  \"$classname$\",\n"
@@ -305,8 +304,7 @@ GenerateMessageDescriptor(io::Printer* printer) {
     "  $n_fields$,\n"
     "  $lcclassname$__field_descriptors,\n"
     "  $n_ranges$,"
-    "  $lcclassname$__number_ranges,\n"
-    "  PROTOBUF_C_OFFSETOF($classname$, unknown_field_array)\n"
+    "  $lcclassname$__number_ranges\n"
     "};\n");
 }
 

src/google/protobuf/compiler/c/c_service.cc

@@ -131,6 +131,7 @@ void ServiceGenerator::GenerateServiceDescriptor(io::Printer* printer)
   printer->Print(vars_, "};\n");
   printer->Print(vars_, "const ProtobufCServiceDescriptor $lcfullname$__descriptor =\n"
                        "{\n"
+		       "  PROTOBUF_C_SERVICE_DESCRIPTOR_MAGIC,\n"
 		       "  \"$fullname$\",\n"
 		       "  \"$name$\",\n"
 		       "  \"$cname$\",\n"

src/test/cxx-generate-packed-data.cc

@@ -47,11 +47,29 @@ dump_test_enum_big (void)
   eb.set_test(VALUE2097152); dump_message_bytes(&eb, "test_enum_big_VALUE2097152");
   eb.set_test(VALUE268435455); dump_message_bytes(&eb, "test_enum_big_VALUE268435455");
   eb.set_test(VALUE268435456); dump_message_bytes(&eb, "test_enum_big_VALUE268435456");
-  }
+}
 
+static void
+dump_test_field_numbers (void)
+{
+#define DUMP_ONE(num) \
+  { TestFieldNo##num f; \
+    f.set_test("tst"); \
+    dump_message_bytes(&f, "test_field_number_" #num); }
+  DUMP_ONE (15)
+  DUMP_ONE (16)
+  DUMP_ONE (2047)
+  DUMP_ONE (2048)
+  DUMP_ONE (262143)
+  DUMP_ONE (262144)
+  DUMP_ONE (33554431)
+  DUMP_ONE (33554432)
+#undef DUMP_ONE
+} 
 int main(int argc, char **argv)
 {
   dump_test_enum_small ();
   dump_test_enum_big ();
+  dump_test_field_numbers ();
   return 0;
 }

src/test/test-generated-code.c

@@ -5,15 +5,13 @@
 
 int main(int argc, char **argv)
 {
-  Foo__Person person;
+  Foo__Person person = FOO__PERSON__INIT;
   Foo__Person *person2;
   unsigned char simple_pad[8];
   size_t size, size2;
   unsigned char *packed;
   ProtobufCBufferSimple bs = PROTOBUF_C_BUFFER_SIMPLE_INIT (simple_pad);
 
-  memset (&person, 0, sizeof (person));
-  person.descriptor = &foo__person__descriptor;
   person.name = "dave b";
   person.id = 42;
   size = foo__person__get_packed_size (&person);

src/test/test-generated-code2.c

@@ -55,6 +55,8 @@ static void test_enum_small (void)
   uint8_t *data;
   Foo__TestMessRequiredEnumSmall *unpacked;
 
+  assert (sizeof (Foo__TestEnumSmall) == 4);
+
   small.test = FOO__TEST_ENUM_SMALL__VALUE;
   unpacked = test_compare_pack_methods ((ProtobufCMessage*)&small, &len, &data);
   assert (unpacked->test == FOO__TEST_ENUM_SMALL__VALUE);
@@ -75,7 +77,9 @@ static void test_enum_big (void)
   Foo__TestMessRequiredEnum big = FOO__TEST_MESS_REQUIRED_ENUM__INIT;
   size_t len;
   uint8_t *data;
-  Foo__TestMessRequiredEnumSmall *unpacked;
+  Foo__TestMessRequiredEnum *unpacked;
+
+  assert (sizeof (Foo__TestEnum) == 4);
 
 #define DO_ONE_TEST(shortname, numeric_value, encoded_len) \
   do{ \
@@ -101,6 +105,33 @@ static void test_enum_big (void)
 
 #undef DO_ONE_TEST
 }
+static void test_field_numbers (void)
+{
+  size_t len;
+  uint8_t *data;
+
+#define DO_ONE_TEST(num, exp_len) \
+  { \
+    Foo__TestFieldNo##num t = FOO__TEST_FIELD_NO##num##__INIT; \
+    Foo__TestFieldNo##num *t2; \
+    t.test = "tst"; \
+    t2 = test_compare_pack_methods ((ProtobufCMessage*)(&t), &len, &data); \
+    assert (strcmp (t2->test, "tst") == 0); \
+    TEST_VERSUS_STATIC_ARRAY (len, data, test_field_number_##num); \
+    assert (len == exp_len); \
+    free (data); \
+    foo__test_field_no##num##__free_unpacked (t2, NULL); \
+  }
+  DO_ONE_TEST (15, 1 + 1 + 3);
+  DO_ONE_TEST (16, 2 + 1 + 3);
+  DO_ONE_TEST (2047, 2 + 1 + 3);
+  DO_ONE_TEST (2048, 3 + 1 + 3);
+  DO_ONE_TEST (262143, 3 + 1 + 3);
+  DO_ONE_TEST (262144, 4 + 1 + 3);
+  DO_ONE_TEST (33554431, 4 + 1 + 3);
+  DO_ONE_TEST (33554432, 5 + 1 + 3);
+#undef DO_ONE_TEST
+}
 
 /* === simple testing framework === */
 
@@ -115,7 +146,7 @@ static Test tests[] =
 {
   { "small enums", test_enum_small },
   { "big enums", test_enum_big },
-  //{ "test field numbers", test_field_numbers },
+  { "test field numbers", test_field_numbers },
   //{ "test repeated int32" ,test_repeated_int32 },
   //{ "test repeated sint32" ,test_repeated_sint32 },
   //{ "test repeated sfixed32" ,test_repeated_sfixed32 },