Skip to content

Commit

Permalink
Remove error erasures (#8272)
Browse files Browse the repository at this point in the history
* Remove error erasures

#### Problem

Various code of the form

    error = chipf();
    Verify(error != CHIP_NO_ERROR, error = CHIP_OTHER_ERROR)

simply discards an error and replaces it with another, usually
CHIP_ERROR_INTERNAL. This has two problems:

- on small platforms, it uses extra code;
- on large platforms, it is planned that the CHIP_ERROR type will
  track the original source location of the error, and throwing
  this away will impede troubleshooting.

#### Change overview

Use the original error code instead of replacing it.

#### Testing

No affected tests check for specific error codes. Unit tests should
be expected to catch any unintended changes to functionality.

* build fixes

* build fixes
  • Loading branch information
kpschoedel authored and pull[bot] committed Aug 5, 2021
1 parent f6823ca commit 5239937
Show file tree
Hide file tree
Showing 10 changed files with 116 additions and 221 deletions.
4 changes: 2 additions & 2 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,11 +543,11 @@ void InitServer(AppDelegate * delegate)

// Register to receive unsolicited legacy ZCL messages from the exchange manager.
err = gExchangeMgr.RegisterUnsolicitedMessageHandlerForProtocol(Protocols::TempZCL::Id, &gCallbacks);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_NO_UNSOLICITED_MESSAGE_HANDLER);
SuccessOrExit(err);

// Register to receive unsolicited Service Provisioning messages from the exchange manager.
err = gExchangeMgr.RegisterUnsolicitedMessageHandlerForProtocol(Protocols::ServiceProvisioning::Id, &gCallbacks);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_NO_UNSOLICITED_MESSAGE_HANDLER);
SuccessOrExit(err);

err = gCASEServer.ListenForSessionEstablishment(&gExchangeMgr, &gTransports, &gSessions, &GetGlobalAdminPairingTable(),
&gSessionIDAllocator);
Expand Down
10 changes: 5 additions & 5 deletions src/ble/BLEEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1428,7 +1428,7 @@ bool BLEEndPoint::SendIndication(PacketBufferHandle && buf)
CHIP_ERROR BLEEndPoint::StartConnectTimer()
{
const CHIP_ERROR timerErr = mBle->mSystemLayer->StartTimer(BLE_CONNECT_TIMEOUT_MS, HandleConnectTimeout, this);
VerifyOrReturnError(timerErr == CHIP_NO_ERROR, BLE_ERROR_START_TIMER_FAILED);
ReturnErrorOnFailure(timerErr);
mTimerStateFlags.Set(TimerStateFlag::kConnectTimerRunning);

return CHIP_NO_ERROR;
Expand All @@ -1437,7 +1437,7 @@ CHIP_ERROR BLEEndPoint::StartConnectTimer()
CHIP_ERROR BLEEndPoint::StartReceiveConnectionTimer()
{
const CHIP_ERROR timerErr = mBle->mSystemLayer->StartTimer(BLE_CONNECT_TIMEOUT_MS, HandleReceiveConnectionTimeout, this);
VerifyOrReturnError(timerErr == CHIP_NO_ERROR, BLE_ERROR_START_TIMER_FAILED);
ReturnErrorOnFailure(timerErr);
mTimerStateFlags.Set(TimerStateFlag::kReceiveConnectionTimerRunning);

return CHIP_NO_ERROR;
Expand All @@ -1448,7 +1448,7 @@ CHIP_ERROR BLEEndPoint::StartAckReceivedTimer()
if (!mTimerStateFlags.Has(TimerStateFlag::kAckReceivedTimerRunning))
{
const CHIP_ERROR timerErr = mBle->mSystemLayer->StartTimer(BTP_ACK_RECEIVED_TIMEOUT_MS, HandleAckReceivedTimeout, this);
VerifyOrReturnError(timerErr == CHIP_NO_ERROR, BLE_ERROR_START_TIMER_FAILED);
ReturnErrorOnFailure(timerErr);

mTimerStateFlags.Set(TimerStateFlag::kAckReceivedTimerRunning);
}
Expand All @@ -1473,7 +1473,7 @@ CHIP_ERROR BLEEndPoint::StartSendAckTimer()
{
ChipLogDebugBleEndPoint(Ble, "starting new SendAckTimer");
const CHIP_ERROR timerErr = mBle->mSystemLayer->StartTimer(BTP_ACK_SEND_TIMEOUT_MS, HandleSendAckTimeout, this);
VerifyOrReturnError(timerErr == CHIP_NO_ERROR, BLE_ERROR_START_TIMER_FAILED);
ReturnErrorOnFailure(timerErr);

mTimerStateFlags.Set(TimerStateFlag::kSendAckTimerRunning);
}
Expand All @@ -1484,7 +1484,7 @@ CHIP_ERROR BLEEndPoint::StartSendAckTimer()
CHIP_ERROR BLEEndPoint::StartUnsubscribeTimer()
{
const CHIP_ERROR timerErr = mBle->mSystemLayer->StartTimer(BLE_UNSUBSCRIBE_TIMEOUT_MS, HandleUnsubscribeTimeout, this);
VerifyOrReturnError(timerErr == CHIP_NO_ERROR, BLE_ERROR_START_TIMER_FAILED);
ReturnErrorOnFailure(timerErr);
mTimerStateFlags.Set(TimerStateFlag::kUnsubscribeTimerRunning);

return CHIP_NO_ERROR;
Expand Down
12 changes: 8 additions & 4 deletions src/ble/BtpEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
mRxCharCount++;

// Get header flags, always in first byte.
VerifyOrExit(reader.Read8(rx_flags.RawStorage()).StatusCode() == CHIP_NO_ERROR, err = CHIP_ERROR_MESSAGE_INCOMPLETE);
err = reader.Read8(rx_flags.RawStorage()).StatusCode();
SuccessOrExit(err);
#if CHIP_ENABLE_CHIPOBLE_TEST
if (rx_flags.Has(HeaderFlags::kCommandMessage))
SetRxPacketType(kType_Control);
Expand All @@ -261,14 +262,16 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
// Get ack number, if any.
if (didReceiveAck)
{
VerifyOrExit(reader.Read8(&receivedAck).StatusCode() == CHIP_NO_ERROR, err = CHIP_ERROR_MESSAGE_INCOMPLETE);
err = reader.Read8(&receivedAck).StatusCode();
SuccessOrExit(err);

err = HandleAckReceived(receivedAck);
SuccessOrExit(err);
}

// Get sequence number.
VerifyOrExit(reader.Read8(&mRxNewestUnackedSeqNum).StatusCode() == CHIP_NO_ERROR, err = CHIP_ERROR_MESSAGE_INCOMPLETE);
err = reader.Read8(&mRxNewestUnackedSeqNum).StatusCode();
SuccessOrExit(err);

// Verify that received sequence number is the next one we'd expect.
VerifyOrExit(mRxNewestUnackedSeqNum == mRxNextSeqNum, err = BLE_ERROR_INVALID_BTP_SEQUENCE_NUMBER);
Expand Down Expand Up @@ -304,7 +307,8 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
// Verify StartMessage header flag set.
VerifyOrExit(rx_flags.Has(HeaderFlags::kStartMessage), err = BLE_ERROR_INVALID_BTP_HEADER_FLAGS);

VerifyOrExit(startReader.Read16(&mRxLength).StatusCode() == CHIP_NO_ERROR, err = CHIP_ERROR_MESSAGE_INCOMPLETE);
err = startReader.Read16(&mRxLength).StatusCode();
SuccessOrExit(err);

mRxState = kState_InProgress;

Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ CHIP_ERROR Device::Deserialize(const SerializedDevice & input)
#if CHIP_SYSTEM_CONFIG_USE_LWIP
UNLOCK_TCPIP_CORE();
#endif
VerifyOrReturnError(CHIP_NO_ERROR == inetErr, CHIP_ERROR_INTERNAL);
ReturnErrorOnFailure(inetErr);
}

static_assert(std::is_same<std::underlying_type<decltype(mDeviceAddress.GetTransportType())>::type, uint8_t>::value,
Expand Down
8 changes: 4 additions & 4 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,8 @@ CHIP_ERROR DeviceController::Shutdown()
//
ReturnErrorOnFailure(DeviceLayer::PlatformMgr().Shutdown());
#else
VerifyOrReturnError(mInetLayer->Shutdown() == CHIP_NO_ERROR, CHIP_ERROR_INTERNAL);
VerifyOrReturnError(mSystemLayer->Shutdown() == CHIP_NO_ERROR, CHIP_ERROR_INTERNAL);
ReturnErrorOnFailure(mInetLayer->Shutdown());
ReturnErrorOnFailure(mSystemLayer->Shutdown());
chip::Platform::Delete(mInetLayer);
chip::Platform::Delete(mSystemLayer);
#endif // CONFIG_DEVICE_LAYER
Expand Down Expand Up @@ -894,13 +894,13 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam
// If the CSRNonce is passed in, using that else using a random one..
if (params.HasCSRNonce())
{
VerifyOrReturnError(device->SetCSRNonce(params.GetCSRNonce().Value()) == CHIP_NO_ERROR, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(device->SetCSRNonce(params.GetCSRNonce().Value()));
}
else
{
uint8_t mCSRNonce[kOpCSRNonceLength];
Crypto::DRBG_get_bytes(mCSRNonce, sizeof(mCSRNonce));
VerifyOrReturnError(device->SetCSRNonce(ByteSpan(mCSRNonce)) == CHIP_NO_ERROR, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(device->SetCSRNonce(ByteSpan(mCSRNonce)));
}

mIsIPRendezvous = (params.GetPeerAddress().GetTransportType() != Transport::Type::kBle);
Expand Down
2 changes: 1 addition & 1 deletion src/credentials/CHIPCertFromX509.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ static CHIP_ERROR ConvertExtension(ASN1Reader & reader, TLVWriter & writer)
ASN1_PARSE_ENTER_SEQUENCE
{
err = reader.Next();
VerifyOrExit(err == CHIP_NO_ERROR, err = ASN1_ERROR_INVALID_ENCODING);
SuccessOrExit(err);

// keyIdentifier [0] IMPLICIT KeyIdentifier,
// KeyIdentifier ::= OCTET STRING
Expand Down
Loading

0 comments on commit 5239937

Please sign in to comment.