Skip to content

Commit 450a037

Browse files
bzbarsky-applehawkhan
bzbarsky-apple
authored and
hawkhan
committed
Add a lint for SuccessOrExit without assignment. (project-chip#26854)
* Add a lint for SuccessOrExit without assignment. This is almost always a mistake, since it loses track of the error involved. * Address review comment.
1 parent 23072a4 commit 450a037

24 files changed

+607
-261
lines changed

‎.github/workflows/lint.yml

+217-163
Large diffs are not rendered by default.

‎examples/platform/nxp/se05x/DeviceAttestationSe05xCredsExample_v2.cpp

+10-9
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ CHIP_ERROR ExampleSe05xDACProviderv2::SignWithDeviceAttestationKey(const ByteSpa
196196
};
197197

198198
tempBuf[0] = (uint8_t) TLV::TLVElementType::Structure;
199-
SuccessOrExit(se05xSetCertificate(START_CONTAINER_SE05X_ID, tempBuf, 1));
199+
SuccessOrExit(err = se05xSetCertificate(START_CONTAINER_SE05X_ID, tempBuf, 1));
200200

201201
for (int i = 1; i <= NO_OF_DEV_ATTEST_MSG_TAGS_TO_PARSE; i++)
202202
{
@@ -206,6 +206,7 @@ CHIP_ERROR ExampleSe05xDACProviderv2::SignWithDeviceAttestationKey(const ByteSpa
206206
{
207207
continue;
208208
}
209+
// TODO: Should this be setting err = tlverr?
209210
SuccessOrExit(tlverr);
210211

211212
// Transient binary object ids starting from location 0x7D300005 (TAG1_ID) to 0x7D30000D (TAG3_VALUE_ID)
@@ -215,35 +216,35 @@ CHIP_ERROR ExampleSe05xDACProviderv2::SignWithDeviceAttestationKey(const ByteSpa
215216
taglen = tagReader.GetLength();
216217
tempBuf[0] = tagReader.GetControlByte();
217218
tempBuf[1] = i;
218-
SuccessOrExit(se05xSetCertificate(TAG1_ID + (3 /* tag + length + value ids */ * (i - 1)), tempBuf, 2));
219+
SuccessOrExit(err = se05xSetCertificate(TAG1_ID + (3 /* tag + length + value ids */ * (i - 1)), tempBuf, 2));
219220
if (taglen > 256)
220221
{
221222
tempBuf[0] = taglen & 0xFF;
222223
tempBuf[1] = (taglen >> 8) & 0xFF;
223-
SuccessOrExit(se05xSetCertificate(TAG1_LEN_ID + (3 * (i - 1)), tempBuf, 2));
224+
SuccessOrExit(err = se05xSetCertificate(TAG1_LEN_ID + (3 * (i - 1)), tempBuf, 2));
224225
}
225226
else
226227
{
227228
tempBuf[0] = taglen;
228-
SuccessOrExit(se05xSetCertificate(TAG1_LEN_ID + (3 * (i - 1)), tempBuf, 1));
229+
SuccessOrExit(err = se05xSetCertificate(TAG1_LEN_ID + (3 * (i - 1)), tempBuf, 1));
229230
}
230231
if (taglen > 0)
231232
{
232-
SuccessOrExit(tagReader.Get(tagvalue));
233-
SuccessOrExit(se05xSetCertificate(TAG1_VALUE_ID + (3 * (i - 1)), tagvalue.data(), taglen));
233+
SuccessOrExit(err = tagReader.Get(tagvalue));
234+
SuccessOrExit(err = se05xSetCertificate(TAG1_VALUE_ID + (3 * (i - 1)), tagvalue.data(), taglen));
234235
}
235236
}
236237

237238
tempBuf[0] = (uint8_t) TLV::TLVElementType::EndOfContainer;
238-
SuccessOrExit(se05xSetCertificate(END_CONTAINER_SE05X_ID, tempBuf, 1));
239+
SuccessOrExit(err = se05xSetCertificate(END_CONTAINER_SE05X_ID, tempBuf, 1));
239240

240241
if ((tagReader.GetRemainingLength() + 1 /* End container */) >= 16)
241242
{
242243
/* Set attestation challenge */
243-
SuccessOrExit(se05xSetCertificate(ATTEST_CHALLENGE_ID, (message_to_sign.end() - 16), 16));
244+
SuccessOrExit(err = se05xSetCertificate(ATTEST_CHALLENGE_ID, (message_to_sign.end() - 16), 16));
244245
}
245246

246-
SuccessOrExit(se05xPerformInternalSign(DEV_ATTESTATION_KEY_SE05X_ID_IS, signature_se05x, &signature_se05x_len));
247+
SuccessOrExit(err = se05xPerformInternalSign(DEV_ATTESTATION_KEY_SE05X_ID_IS, signature_se05x, &signature_se05x_len));
247248

248249
err = chip::Crypto::EcdsaAsn1SignatureToRaw(chip::Crypto::kP256_FE_Length, ByteSpan{ signature_se05x, signature_se05x_len },
249250
out_signature_buffer);

‎src/access/AccessControl.cpp

+16-8
Original file line numberDiff line numberDiff line change
@@ -555,11 +555,12 @@ bool AccessControl::IsValid(const Entry & entry)
555555
size_t subjectCount = 0;
556556
size_t targetCount = 0;
557557

558-
SuccessOrExit(entry.GetAuthMode(authMode));
559-
SuccessOrExit(entry.GetFabricIndex(fabricIndex));
560-
SuccessOrExit(entry.GetPrivilege(privilege));
561-
SuccessOrExit(entry.GetSubjectCount(subjectCount));
562-
SuccessOrExit(entry.GetTargetCount(targetCount));
558+
CHIP_ERROR err = CHIP_NO_ERROR;
559+
SuccessOrExit(err = entry.GetAuthMode(authMode));
560+
SuccessOrExit(err = entry.GetFabricIndex(fabricIndex));
561+
SuccessOrExit(err = entry.GetPrivilege(privilege));
562+
SuccessOrExit(err = entry.GetSubjectCount(subjectCount));
563+
SuccessOrExit(err = entry.GetTargetCount(targetCount));
563564

564565
#if CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY > 1
565566
ChipLogProgress(DataManagement, "AccessControl: validating f=%u p=%c a=%c s=%d t=%d", fabricIndex,
@@ -582,7 +583,7 @@ bool AccessControl::IsValid(const Entry & entry)
582583
for (size_t i = 0; i < subjectCount; ++i)
583584
{
584585
NodeId subject;
585-
SuccessOrExit(entry.GetSubject(i, subject));
586+
SuccessOrExit(err = entry.GetSubject(i, subject));
586587
const bool kIsCase = authMode == AuthMode::kCase;
587588
const bool kIsGroup = authMode == AuthMode::kGroup;
588589
#if CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY > 1
@@ -594,7 +595,7 @@ bool AccessControl::IsValid(const Entry & entry)
594595
for (size_t i = 0; i < targetCount; ++i)
595596
{
596597
Entry::Target target;
597-
SuccessOrExit(entry.GetTarget(i, target));
598+
SuccessOrExit(err = entry.GetTarget(i, target));
598599
const bool kHasCluster = target.flags & Entry::Target::kCluster;
599600
const bool kHasEndpoint = target.flags & Entry::Target::kEndpoint;
600601
const bool kHasDeviceType = target.flags & Entry::Target::kDeviceType;
@@ -608,7 +609,14 @@ bool AccessControl::IsValid(const Entry & entry)
608609
return true;
609610

610611
exit:
611-
ChipLogError(DataManagement, "AccessControl: %s", log);
612+
if (err != CHIP_NO_ERROR)
613+
{
614+
ChipLogError(DataManagement, "AccessControl: %s %" CHIP_ERROR_FORMAT, log, err.Format());
615+
}
616+
else
617+
{
618+
ChipLogError(DataManagement, "AccessControl: %s", log);
619+
}
612620
return false;
613621
}
614622

‎src/app/CommandHandler.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem
324324
{
325325
ChipLogDetail(DataManagement, "Received command for Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI,
326326
concretePath.mEndpointId, ChipLogValueMEI(concretePath.mClusterId), ChipLogValueMEI(concretePath.mCommandId));
327-
SuccessOrExit(MatterPreCommandReceivedCallback(concretePath, GetSubjectDescriptor()));
327+
SuccessOrExit(err = MatterPreCommandReceivedCallback(concretePath, GetSubjectDescriptor()));
328328
mpCallback->DispatchCommand(*this, concretePath, commandDataReader);
329329
MatterPostCommandReceivedCallback(concretePath, GetSubjectDescriptor());
330330
}

‎src/app/WriteClient.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchang
460460
if (!mChunks.IsNull())
461461
{
462462
// Send the next chunk.
463-
SuccessOrExit(SendWriteRequest());
463+
SuccessOrExit(err = SendWriteRequest());
464464
}
465465
}
466466
else if (aPayloadHeader.HasMessageType(MsgType::StatusResponse))

‎src/app/reporting/Engine.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
529529
}
530530
}
531531

532-
SuccessOrExit(reportDataBuilder.GetError());
532+
SuccessOrExit(err = reportDataBuilder.GetError());
533533
SuccessOrExit(err = reportDataWriter.UnreserveBuffer(kReservedSizeForMoreChunksFlag + kReservedSizeForIMRevision +
534534
kReservedSizeForEndOfReportMessage));
535535
if (hasMoreChunks)

‎src/app/server/Server.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
143143

144144
// Set up attribute persistence before we try to bring up the data model
145145
// handler.
146-
SuccessOrExit(mAttributePersister.Init(mDeviceStorage));
146+
SuccessOrExit(err = mAttributePersister.Init(mDeviceStorage));
147147
SetAttributePersistenceProvider(&mAttributePersister);
148148

149149
{

‎src/app/tests/integration/chip_im_initiator.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ CHIP_ERROR SendReadRequest()
347347
chip::Platform::MakeUnique<chip::app::ReadClient>(chip::app::InteractionModelEngine::GetInstance(), &gExchangeManager,
348348
gMockDelegate, chip::app::ReadClient::InteractionType::Read);
349349

350-
SuccessOrExit(readClient->SendRequest(readPrepareParams));
350+
SuccessOrExit(err = readClient->SendRequest(readPrepareParams));
351351

352352
gMockDelegate.AdoptReadClient(std::move(readClient));
353353

‎src/controller/java/AndroidCommissioningWindowOpener.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ void AndroidCommissioningWindowOpener::OnOpenCommissioningWindowResponse(void *
121121
std::string QRCode;
122122
std::string manualPairingCode;
123123

124-
SuccessOrExit(ManualSetupPayloadGenerator(payload).payloadDecimalStringRepresentation(manualPairingCode));
125-
SuccessOrExit(QRCodeSetupPayloadGenerator(payload).payloadBase38Representation(QRCode));
124+
SuccessOrExit(status = ManualSetupPayloadGenerator(payload).payloadDecimalStringRepresentation(manualPairingCode));
125+
SuccessOrExit(status = QRCodeSetupPayloadGenerator(payload).payloadBase38Representation(QRCode));
126126

127127
if (self->mOnSuccessMethod != nullptr)
128128
{

0 commit comments

Comments
 (0)