[ObjC] Fix GPBUnknownField/GPBUnknownFields copy.

Since a group GPBUnknownField uses a GPBUnknownFields and that is mutable, it needs to be copied so two instances aren't linked.

PiperOrigin-RevId: 663323972
pull/17824/head
Thomas Van Lenten 7 months ago committed by Copybara-Service
parent c5f623184d
commit 7155a7e95d
  1. 10
      objectivec/GPBUnknownField.m
  2. 5
      objectivec/GPBUnknownFields.m
  3. 25
      objectivec/Tests/GPBUnknownFieldsTest.m

@ -165,9 +165,17 @@
case GPBUnknownFieldTypeFixed32:
case GPBUnknownFieldTypeFixed64:
case GPBUnknownFieldTypeLengthDelimited:
case GPBUnknownFieldTypeGroup:
// In these modes, the object isn't mutable, so just return self.
return [self retain];
case GPBUnknownFieldTypeGroup: {
// The `GPBUnknownFields` for the group is mutable, so a new instance of this object and
// the group is needed.
GPBUnknownFields *copyGroup = [storage_.group copyWithZone:zone];
GPBUnknownField *copy = [[GPBUnknownField allocWithZone:zone] initWithNumber:number_
group:copyGroup];
[copyGroup release];
return copy;
}
case GPBUnknownFieldTypeLegacy: {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"

@ -228,9 +228,8 @@ static BOOL MergeFromInputStream(GPBUnknownFields *self, GPBCodedInputStream *in
}
- (id)copyWithZone:(NSZone *)zone {
GPBUnknownFields *copy = [[GPBUnknownFields alloc] init];
// Fields are r/o in this api, so just copy the array.
copy->fields_ = [fields_ mutableCopyWithZone:zone];
GPBUnknownFields *copy = [[GPBUnknownFields allocWithZone:zone] init];
copy->fields_ = [[NSMutableArray allocWithZone:zone] initWithArray:fields_ copyItems:YES];
return copy;
}

@ -683,25 +683,30 @@
[ufs addFieldNumber:3 fixed64:3];
[ufs addFieldNumber:4 lengthDelimited:DataFromCStr("foo")];
GPBUnknownFields* group = [ufs addGroupWithFieldNumber:5];
[group addFieldNumber:10 varint:10];
GPBUnknownFields* subGroup = [group addGroupWithFieldNumber:100];
[subGroup addFieldNumber:20 varint:20];
GPBUnknownFields* ufs2 = [[ufs copy] autorelease];
XCTAssertTrue(ufs != ufs2); // Different objects
XCTAssertEqualObjects(ufs, ufs2); // Equal contents
// All the actual field objects should be the same since they are immutable.
// All field objects but the group should be the same since they are immutable.
XCTAssertTrue([[ufs fields:1] firstObject] == [[ufs2 fields:1] firstObject]); // Same object
XCTAssertTrue([[ufs fields:2] firstObject] == [[ufs2 fields:2] firstObject]); // Same object
XCTAssertTrue([[ufs fields:3] firstObject] == [[ufs2 fields:3] firstObject]); // Same object
XCTAssertTrue([[ufs fields:4] firstObject] == [[ufs2 fields:4] firstObject]); // Same object
XCTAssertTrue([[ufs fields:4] firstObject].lengthDelimited ==
[[ufs2 fields:4] firstObject].lengthDelimited); // Same object
XCTAssertTrue([[ufs fields:5] firstObject] == [[ufs2 fields:5] firstObject]); // Same object
XCTAssertTrue(group == [[ufs2 fields:5] firstObject].group); // Same object
// Now force copies on the fields to confirm that is not making new objects either.
for (GPBUnknownField* field in ufs) {
GPBUnknownField* field2 = [[field copy] autorelease];
XCTAssertTrue(field == field2); // Same object (since they aren't mutable).
}
[[ufs2 fields:4] firstObject].lengthDelimited); // Same object
// Since the group holds another `GPBUnknownFields` object (which is mutable), it will be a
// different object.
XCTAssertTrue([[ufs fields:5] firstObject] != [[ufs2 fields:5] firstObject]);
XCTAssertTrue(group != [[ufs2 fields:5] firstObject].group);
XCTAssertEqualObjects(group, [[ufs2 fields:5] firstObject].group);
// And confirm that copy went deep so the nested group also is a different object.
GPBUnknownFields* groupCopied = [[ufs2 fields:5] firstObject].group;
XCTAssertTrue([[group fields:100] firstObject] != [[groupCopied fields:100] firstObject]);
XCTAssertTrue(subGroup != [[groupCopied fields:100] firstObject].group);
XCTAssertEqualObjects(subGroup, [[groupCopied fields:100] firstObject].group);
}
- (void)testInvalidFieldNumbers {

Loading…
Cancel
Save