From 2a46af42784abf86804d536f6e0122d47cfeea45 Mon Sep 17 00:00:00 2001 From: Paolo Borelli Date: Fri, 26 May 2017 18:24:32 +0200 Subject: [PATCH] proto3: make strings default to "" instead of NULL The spec talks about "empty string" and other languages like C# return a zero length string and not null. This is useful because when moving from proto2's "required string" to a proto3's plain string we will be guaranteed that we never get a null pointer. The tradeoff is adding a special case to the library but avoiding a lot of null checks in the calling code. The current code is already special casing "" in pack_string. --- protobuf-c/libprotobuf-c.sym | 5 +++++ protobuf-c/protobuf-c.c | 5 +++++ protobuf-c/protobuf-c.h | 3 +++ protoc-c/c_field.cc | 3 +++ protoc-c/c_string_field.cc | 4 +++- t/generated-code/test-generated-code.c | 5 +++++ 6 files changed, 24 insertions(+), 1 deletion(-) diff --git a/protobuf-c/libprotobuf-c.sym b/protobuf-c/libprotobuf-c.sym index e69c1fc..b72e885 100644 --- a/protobuf-c/libprotobuf-c.sym +++ b/protobuf-c/libprotobuf-c.sym @@ -21,3 +21,8 @@ global: local: *; }; + +LIBPROTOBUF_C_1.3.0 { +global: + protobuf_c_empty_string; +} LIBPROTOBUF_C_1.0.0; diff --git a/protobuf-c/protobuf-c.c b/protobuf-c/protobuf-c.c index b26bf02..5debac8 100644 --- a/protobuf-c/protobuf-c.c +++ b/protobuf-c/protobuf-c.c @@ -84,6 +84,8 @@ # define PROTOBUF_C_UNPACK_ERROR(...) #endif +const char protobuf_c_empty_string[] = ""; + /** * Internal `ProtobufCMessage` manipulation macro. * @@ -554,6 +556,9 @@ field_is_zeroish(const ProtobufCFieldDescriptor *field, ret = (0 == *(const double *) member); break; case PROTOBUF_C_TYPE_STRING: + ret = (NULL == *(const char * const *) member) || + ('\0' == **(const char * const *) member); + break; case PROTOBUF_C_TYPE_BYTES: case PROTOBUF_C_TYPE_MESSAGE: ret = (NULL == *(const void * const *) member); diff --git a/protobuf-c/protobuf-c.h b/protobuf-c/protobuf-c.h index b39282e..35b2073 100755 --- a/protobuf-c/protobuf-c.h +++ b/protobuf-c/protobuf-c.h @@ -237,6 +237,9 @@ PROTOBUF_C__BEGIN_DECLS #define PROTOBUF_C__MESSAGE_DESCRIPTOR_MAGIC 0x28aaeef9 #define PROTOBUF_C__ENUM_DESCRIPTOR_MAGIC 0x114315af +/* Empty string used for initializers */ +extern const char protobuf_c_empty_string[]; + /** * \defgroup api Public API * diff --git a/protoc-c/c_field.cc b/protoc-c/c_field.cc index 8366d37..0e9b1f9 100644 --- a/protoc-c/c_field.cc +++ b/protoc-c/c_field.cc @@ -129,6 +129,9 @@ void FieldGenerator::GenerateDescriptorInitializerGeneric(io::Printer* printer, variables["default_value"] = string("&") + FullNameToLower(descriptor_->full_name()) + "__default_value"; + } else if (FieldSyntax(descriptor_) == 3 && + descriptor_->type() == FieldDescriptor::TYPE_STRING) { + variables["default_value"] = "&protobuf_c_empty_string"; } else { variables["default_value"] = "NULL"; } diff --git a/protoc-c/c_string_field.cc b/protoc-c/c_string_field.cc index feaa7ae..4075db1 100644 --- a/protoc-c/c_string_field.cc +++ b/protoc-c/c_string_field.cc @@ -125,8 +125,10 @@ void StringFieldGenerator::GenerateStaticInit(io::Printer* printer) const std::map vars; if (descriptor_->has_default_value()) { vars["default"] = GetDefaultValue(); - } else { + } else if (FieldSyntax(descriptor_) == 2) { vars["default"] = "NULL"; + } else { + vars["default"] = "(char *)protobuf_c_empty_string"; } switch (descriptor_->label()) { case FieldDescriptor::LABEL_REQUIRED: diff --git a/t/generated-code/test-generated-code.c b/t/generated-code/test-generated-code.c index 7443a09..ab5660d 100644 --- a/t/generated-code/test-generated-code.c +++ b/t/generated-code/test-generated-code.c @@ -51,6 +51,11 @@ int main(void) person2 = foo__person__unpack (NULL, size, packed); assert (person2 != NULL); assert (person2->id == 42); +#ifndef PROTO3 + assert (person2->email == NULL); +#else + assert (strcmp (person2->email, "") == 0); +#endif assert (strcmp (person2->name, "dave b") == 0); assert (person2->n_phone == 1); assert (strcmp (person2->phone[0]->number, "1234") == 0);