added check for PROTOBUF_C_TYPE_BYTES fields, including repeated

added check that repeated fields vectors are not NULL
fixed repeated field quantity type: it's "size_t", not "unsigned"
cleaner code, no cast porn
all covered with tests

protobuf-c/protobuf-c.c

@@ -3049,43 +3049,66 @@ protobuf_c_message_check(const ProtobufCMessage *message)
 		return FALSE;
 	}
 
-	unsigned f;
-	for (f = 0; f < message->descriptor->n_fields; f++) {
-		ProtobufCType type = message->descriptor->fields[f].type;
-		ProtobufCLabel label = message->descriptor->fields[f].label;
-		if (type == PROTOBUF_C_TYPE_MESSAGE) {
-			unsigned offset = message->descriptor->fields[f].offset;
-			if (label == PROTOBUF_C_LABEL_REQUIRED) {
-				ProtobufCMessage *sub = *(ProtobufCMessage **)
-					(void *)((char *) message + offset);
-				if (sub == NULL)
-					return FALSE;
-				if (!protobuf_c_message_check(sub))
-					return FALSE;
-			} else if (label == PROTOBUF_C_LABEL_OPTIONAL) {
-				ProtobufCMessage *sub = *(ProtobufCMessage **)
-					(void *)((char *) message + offset);
-				if (sub != NULL && !protobuf_c_message_check(sub))
-					return FALSE;
-			} else if (label == PROTOBUF_C_LABEL_REPEATED) {
-				unsigned n = *(unsigned *)(void *)((char *) message +
-					message->descriptor->fields[f].quantifier_offset);
-				ProtobufCMessage **subs = *(ProtobufCMessage ***)
-					(void *)((char *) message + offset);
 	unsigned i;
-				for (i = 0; i < n; i++)
-					if (!protobuf_c_message_check(subs[i]))
+	for (i = 0; i < message->descriptor->n_fields; i++) {
+		const ProtobufCFieldDescriptor *f = message->descriptor->fields + i;
+		ProtobufCType type = f->type;
+		ProtobufCLabel label = f->label;
+		void *field = STRUCT_MEMBER_P (message, f->offset);
+
+		if (label == PROTOBUF_C_LABEL_REPEATED) {
+			size_t *quantity = STRUCT_MEMBER_P (message, f->quantifier_offset);
+
+			if (*quantity > 0 && *(void **) field == NULL) {
+				return FALSE;
+			}
+
+			if (type == PROTOBUF_C_TYPE_MESSAGE) {
+				ProtobufCMessage **submessage = *(ProtobufCMessage ***) field;
+				unsigned j;
+				for (j = 0; j < *quantity; j++) {
+					if (!protobuf_c_message_check(submessage[j]))
 						return FALSE;
 				}
 			} else if (type == PROTOBUF_C_TYPE_STRING) {
-			if (label == PROTOBUF_C_LABEL_REQUIRED) {
-				char *str = *(char **)(void *)((char *)
-					message + message->descriptor->fields[f].offset);
-				if (str == NULL)
+				char **string = *(char ***) field;
+				unsigned j;
+				for (j = 0; j < *quantity; j++) {
+					if (!string[j])
+						return FALSE;
+				}
+			} else if (type == PROTOBUF_C_TYPE_BYTES) {
+				ProtobufCBinaryData *bd = *(ProtobufCBinaryData **) field;
+				unsigned j;
+				for (j = 0; j < *quantity; j++) {
+					if (bd[j].len > 0 && bd[j].data == NULL)
+						return FALSE;
+				}
+			}
+
+		} else { /* PROTOBUF_C_LABEL_REQUIRED or PROTOBUF_C_LABEL_OPTIONAL */
+
+			if (type == PROTOBUF_C_TYPE_MESSAGE) {
+				ProtobufCMessage *submessage = *(ProtobufCMessage **) field;
+				if (label == PROTOBUF_C_LABEL_REQUIRED || submessage != NULL) {
+					if (!protobuf_c_message_check(submessage))
+						return FALSE;
+				}
+			} else if (type == PROTOBUF_C_TYPE_STRING) {
+				char *string = *(char **) field;
+				if (label == PROTOBUF_C_LABEL_REQUIRED && string == NULL)
+					return FALSE;
+			} else if (type == PROTOBUF_C_TYPE_BYTES) {
+				protobuf_c_boolean *has = STRUCT_MEMBER_P (message, f->quantifier_offset);
+				ProtobufCBinaryData *bd = field;
+				if (label == PROTOBUF_C_LABEL_REQUIRED || *has == TRUE) {
+					if (bd->len > 0 && bd->data == NULL)
 						return FALSE;
 				}
 			}
 		}
+	}
+
 	return TRUE;
 }
 

t/generated-code2/test-generated-code2.c

@@ -1648,6 +1648,126 @@ test_field_flags (void)
   assert((f->flags & PROTOBUF_C_FIELD_FLAG_DEPRECATED));
 }
 
+static void
+test_message_check(void)
+{
+  Foo__TestMessageCheck__SubMessage sm = FOO__TEST_MESSAGE_CHECK__SUB_MESSAGE__INIT;
+  Foo__TestMessageCheck__SubMessage sm2 = FOO__TEST_MESSAGE_CHECK__SUB_MESSAGE__INIT;
+  Foo__TestMessageCheck m = FOO__TEST_MESSAGE_CHECK__INIT;
+  char *null = NULL;
+  char *str = "";
+  Foo__TestMessageCheck__SubMessage *sm_p;
+  ProtobufCBinaryData bd;
+
+  /* test incomplete submessage */
+  assert(0 == protobuf_c_message_check(&sm.base));
+
+  /* test complete submessage */
+  sm.str = str;
+  assert(1 == protobuf_c_message_check(&sm.base));
+
+  /* test just initialized message */
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with required_string not set */
+  m.required_string = NULL;
+  m.required_msg = &sm;
+  m.required_bytes.data = str; m.required_bytes.len = 1;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with required_msg not set */
+  m.required_string = str;
+  m.required_msg = NULL;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with required_bytes set incorrectly */
+  m.required_msg = &sm;
+  m.required_bytes.data = NULL; m.required_bytes.len = 1;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with all required fields set */
+  m.required_bytes.data = str; m.required_bytes.len = 1;
+  assert(1 == protobuf_c_message_check(&m.base));
+
+  /* test with incomplete required submessage */
+  sm.str = NULL;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with incomplete optional submessage */
+  sm.str = str;
+  m.optional_msg = &sm2;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with complete optional submessage */
+  sm2.str = str;
+  assert(1 == protobuf_c_message_check(&m.base));
+
+  /* test with correct optional bytes */
+  m.has_optional_bytes = 1;
+  assert(1 == protobuf_c_message_check(&m.base));
+
+  /* test with incorrect optional bytes */
+  m.optional_bytes.data = NULL; m.optional_bytes.len = 1;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with correct optional bytes */
+  m.optional_bytes.data = str; m.optional_bytes.len = 1;
+  assert(1 == protobuf_c_message_check(&m.base));
+
+  /* test with repeated strings set incorrectly */
+  m.n_repeated_string = 1;
+  m.repeated_string = &null;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with repeated strings set correctly */
+  m.repeated_string = &str;
+  assert(1 == protobuf_c_message_check(&m.base));
+
+  /* test with repeated submessage set incorrectly */
+  sm_p = NULL;
+  m.n_repeated_msg = 1;
+  m.repeated_msg = &sm_p;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with repeated incomplete submessage */
+  sm_p = &sm;
+  sm.str = NULL;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with repeated complete submessage */
+  sm.str = str;
+  assert(1 == protobuf_c_message_check(&m.base));
+
+  /* test with repeated bytes set incorrectly */
+  bd.len = 1;
+  bd.data = NULL;
+  m.n_repeated_bytes = 1;
+  m.repeated_bytes = &bd;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with repeated bytes set correctly */
+  bd.data = str;
+  assert(1 == protobuf_c_message_check(&m.base));
+
+  /* test with empty repeated string vector */
+  m.repeated_string = NULL;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with empty repeated bytes vector */
+  m.repeated_string = &str;
+  m.repeated_bytes = NULL;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test with empty repeated submessage vector */
+  m.repeated_bytes = &bd;
+  m.repeated_msg = NULL;
+  assert(0 == protobuf_c_message_check(&m.base));
+
+  /* test all vectors set again */
+  m.repeated_msg = &sm_p;
+  assert(1 == protobuf_c_message_check(&m.base));
+}
+
 /* === simple testing framework === */
 
 typedef void (*TestFunc) (void);
@@ -1756,6 +1876,8 @@ static Test tests[] =
   { "test required_fields_bitmap", test_required_fields_bitmap },
 
   { "test field flags", test_field_flags },
+
+  { "test message_check()", test_message_check },
 };
 #define n_tests (sizeof(tests)/sizeof(Test))
 

t/test-full.proto

@@ -362,3 +362,19 @@ message TestFieldFlags {
   repeated int32 packed_deprecated  = 5 [packed=true, deprecated=true];
   repeated int32 deprecated         = 6 [deprecated=true];
 }
+
+message TestMessageCheck {
+  message SubMessage {
+    required string str = 1;
+  }
+  required SubMessage required_msg  = 1;
+  repeated SubMessage repeated_msg  = 2;
+  optional SubMessage optional_msg  = 3;
+  required string required_string   = 4;
+  repeated string repeated_string   = 5;
+  optional string optional_string   = 6;
+  required bytes required_bytes     = 7;
+  repeated bytes repeated_bytes     = 8;
+  optional bytes optional_bytes     = 9;
+}
+