From 09c001e999660401162b592ef16c59b2036339f1 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Tue, 2 Oct 2018 10:42:55 -0400 Subject: Copy the value when setting message/data fields. Follow ObjC conventions and how the generated header labels things by copying NSStrings/NSData fields when setting them. --- objectivec/GPBMessage.m | 12 ++++++++-- objectivec/GPBUtilities.m | 56 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m index c51dfb25..c8664117 100644 --- a/objectivec/GPBMessage.m +++ b/objectivec/GPBMessage.m @@ -3084,6 +3084,14 @@ static void ResolveIvarSet(__unsafe_unretained GPBFieldDescriptor *field, result->encodingSelector = @selector(set##NAME:); \ break; \ } +#define CASE_SET_COPY(NAME) \ + case GPBDataType##NAME: { \ + result->impToAdd = imp_implementationWithBlock(^(id obj, id value) { \ + return GPBSetRetainedObjectIvarWithFieldInternal(obj, field, [value copy], syntax); \ + }); \ + result->encodingSelector = @selector(set##NAME:); \ + break; \ + } CASE_SET(Bool, BOOL, Bool) CASE_SET(Fixed32, uint32_t, UInt32) CASE_SET(SFixed32, int32_t, Int32) @@ -3097,8 +3105,8 @@ static void ResolveIvarSet(__unsafe_unretained GPBFieldDescriptor *field, CASE_SET(SInt64, int64_t, Int64) CASE_SET(UInt32, uint32_t, UInt32) CASE_SET(UInt64, uint64_t, UInt64) - CASE_SET(Bytes, id, Object) - CASE_SET(String, id, Object) + CASE_SET_COPY(Bytes) + CASE_SET_COPY(String) CASE_SET(Message, id, Object) CASE_SET(Group, id, Object) CASE_SET(Enum, int32_t, Enum) diff --git a/objectivec/GPBUtilities.m b/objectivec/GPBUtilities.m index e2a12ca4..1aedb6cc 100644 --- a/objectivec/GPBUtilities.m +++ b/objectivec/GPBUtilities.m @@ -448,6 +448,36 @@ void GPBMaybeClearOneof(GPBMessage *self, GPBOneofDescriptor *oneof, //% GPBSetObjectIvarWithField(self, field, (id)value); //%} //% +//%PDDM-DEFINE IVAR_ALIAS_DEFN_COPY_OBJECT(NAME, TYPE) +//%// Only exists for public api, no core code should use this. +//%TYPE *GPBGetMessage##NAME##Field(GPBMessage *self, +//% TYPE$S NAME$S GPBFieldDescriptor *field) { +//%#if defined(DEBUG) && DEBUG +//% NSCAssert(DataTypesEquivalent(GPBGetFieldDataType(field), +//% GPBDataType##NAME), +//% @"Attempting to get value of TYPE from field %@ " +//% @"of %@ which is of type %@.", +//% [self class], field.name, +//% TypeToString(GPBGetFieldDataType(field))); +//%#endif +//% return (TYPE *)GPBGetObjectIvarWithField(self, field); +//%} +//% +//%// Only exists for public api, no core code should use this. +//%void GPBSetMessage##NAME##Field(GPBMessage *self, +//% NAME$S GPBFieldDescriptor *field, +//% NAME$S TYPE *value) { +//%#if defined(DEBUG) && DEBUG +//% NSCAssert(DataTypesEquivalent(GPBGetFieldDataType(field), +//% GPBDataType##NAME), +//% @"Attempting to set field %@ of %@ which is of type %@ with " +//% @"value of type TYPE.", +//% [self class], field.name, +//% TypeToString(GPBGetFieldDataType(field))); +//%#endif +//% GPBSetCopyObjectIvarWithField(self, field, (id)value); +//%} +//% // Object types are handled slightly differently, they need to be released // and retained. @@ -483,6 +513,24 @@ static void GPBSetObjectIvarWithField(GPBMessage *self, syntax); } +static void GPBSetCopyObjectIvarWithField(GPBMessage *self, + GPBFieldDescriptor *field, id value); + +// GPBSetCopyObjectIvarWithField is blocked from the analyzer because it flags +// a leak for the -copy even though GPBSetRetainedObjectIvarWithFieldInternal +// is marked as consuming the value. Note: For some reason this doesn't happen +// with the -retain in GPBSetObjectIvarWithField. +#if !defined(__clang_analyzer__) +// This exists only for briging some aliased types, nothing else should use it. +static void GPBSetCopyObjectIvarWithField(GPBMessage *self, + GPBFieldDescriptor *field, id value) { + if (self == nil || field == nil) return; + GPBFileSyntax syntax = [self descriptor].file.syntax; + GPBSetRetainedObjectIvarWithFieldInternal(self, field, [value copy], + syntax); +} +#endif // !defined(__clang_analyzer__) + void GPBSetObjectIvarWithFieldInternal(GPBMessage *self, GPBFieldDescriptor *field, id value, GPBFileSyntax syntax) { @@ -1168,7 +1216,7 @@ void GPBSetDoubleIvarWithFieldInternal(GPBMessage *self, // Aliases are function calls that are virtually the same. -//%PDDM-EXPAND IVAR_ALIAS_DEFN_OBJECT(String, NSString) +//%PDDM-EXPAND IVAR_ALIAS_DEFN_COPY_OBJECT(String, NSString) // This block of code is generated, do not edit it directly. // Only exists for public api, no core code should use this. @@ -1197,10 +1245,10 @@ void GPBSetMessageStringField(GPBMessage *self, [self class], field.name, TypeToString(GPBGetFieldDataType(field))); #endif - GPBSetObjectIvarWithField(self, field, (id)value); + GPBSetCopyObjectIvarWithField(self, field, (id)value); } -//%PDDM-EXPAND IVAR_ALIAS_DEFN_OBJECT(Bytes, NSData) +//%PDDM-EXPAND IVAR_ALIAS_DEFN_COPY_OBJECT(Bytes, NSData) // This block of code is generated, do not edit it directly. // Only exists for public api, no core code should use this. @@ -1229,7 +1277,7 @@ void GPBSetMessageBytesField(GPBMessage *self, [self class], field.name, TypeToString(GPBGetFieldDataType(field))); #endif - GPBSetObjectIvarWithField(self, field, (id)value); + GPBSetCopyObjectIvarWithField(self, field, (id)value); } //%PDDM-EXPAND IVAR_ALIAS_DEFN_OBJECT(Message, GPBMessage) -- cgit v1.2.3