Skip to content

Commit bae572a

Browse files
committed
[Darwin] Address gemini-code-assist review feedback on PR project-chip#72211
Two fixes per project-chip#72211 review: * MTRDeviceAttestationDelegateBridge.mm: switch the attestationUserInfo construction from an NSDictionary literal to a mutable dictionary with nil checks. NSDictionary literal syntax @{key:value} raises NSInvalidArgumentException on a nil value; the @(uint16_t) boxing for the VID/PID NSNumbers always succeeds today, but defensive construction guards against future DeviceInfoForAttestation struct changes that could make those fields nullable. * MTRError.mm: handle Windows-style backslash path separators in errorCode.GetFile() basename stripping. CHIP builds run on Windows for some embedded controller targets; without the fallback to strrchr(file, '\\'), full Windows build-host paths would land in MTRCHIPErrorSourceFileKey, defeating the privacy intent. Both flagged by gemini-code-assist's automated review.
1 parent 2b47e05 commit bae572a

2 files changed

Lines changed: 19 additions & 6 deletions

File tree

src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,19 @@
7878
// certified vendor / product IDs from BasicInformation cluster — partner already
7979
// knows their own VID/PID, but presence in userInfo lets triage tooling correlate
8080
// attestation failures by device-class without parsing logs.
81-
NSDictionary<NSErrorUserInfoKey, id> * attestationUserInfo = @{
82-
MTRAttestationVerificationResultKey : @(chip::to_underlying(attestationResult)),
83-
MTRAttestationDeviceVendorIDKey : basicInformationVendorID,
84-
MTRAttestationDeviceProductIDKey : basicInformationProductID,
85-
};
81+
//
82+
// Use a mutable dictionary with nil checks rather than an NS dictionary literal:
83+
// `@{ key : value }` raises NSInvalidArgumentException if any value is nil. The VID/PID
84+
// NSNumbers are non-nil today (the @(uint16_t) boxing always succeeds), but defensive
85+
// construction guards against future struct changes that make those fields nullable.
86+
NSMutableDictionary<NSErrorUserInfoKey, id> * attestationUserInfo = [NSMutableDictionary dictionary];
87+
attestationUserInfo[MTRAttestationVerificationResultKey] = @(chip::to_underlying(attestationResult));
88+
if (basicInformationVendorID != nil) {
89+
attestationUserInfo[MTRAttestationDeviceVendorIDKey] = basicInformationVendorID;
90+
}
91+
if (basicInformationProductID != nil) {
92+
attestationUserInfo[MTRAttestationDeviceProductIDKey] = basicInformationProductID;
93+
}
8694

8795
id<MTRDeviceAttestationDelegate> strongDelegate = mDeviceAttestationDelegate;
8896
if ([strongDelegate respondsToSelector:@selector(deviceAttestationCompletedForController:opaqueDeviceHandle:attestationDeviceInfo:error:)]

src/darwin/Framework/CHIP/MTRError.mm

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,10 +167,15 @@ + (NSError *)errorForCHIPErrorCode:(CHIP_ERROR)errorCode
167167

168168
// Surface CHIP source-file basename + line when CHIP was built with
169169
// CHIP_CONFIG_ERROR_SOURCE. The full path is intentionally elided to avoid
170-
// exposing build-host paths; only the basename is forwarded.
170+
// exposing build-host paths; only the basename is forwarded. Check both
171+
// forward and backward slashes so basename-stripping works on Windows builds
172+
// as well as Unix-style hosts.
171173
const char * sourceFile = errorCode.GetFile();
172174
if (sourceFile != nullptr && sourceFile[0] != '\0') {
173175
const char * basename = strrchr(sourceFile, '/');
176+
if (basename == nullptr) {
177+
basename = strrchr(sourceFile, '\\');
178+
}
174179
userInfo[MTRCHIPErrorSourceFileKey] = @((basename != nullptr) ? basename + 1 : sourceFile);
175180
userInfo[MTRCHIPErrorSourceLineKey] = @(errorCode.GetLine());
176181
}

0 commit comments

Comments
 (0)