Skip to content

Commit

Permalink
Remove exit: return idiom from some directories (project-chip#7768)
Browse files Browse the repository at this point in the history 
#### Problem

The `Exit` group of error-handling macros inhibit scoping, and
are unnecessary when there is no cleanup at the `exit:` label.

Part of project-chip#7721 _goto exit pattern should be replaced with RAII in C++_

#### Change overview

Remove simple `exit: return` from
  * `src/app`
  * `src/controller`
  * `src/crypto`
  * `src/inet`
  * `src/lib`
  * `src/protocols`
  * `src/setup_payload`
  * `src/transport`
replacing `Exit` macros with corresponding `Return` macros.

Note that this does not change any `exit:` blocks with actual cleanup.

#### Testing

No change to functionality; existing tests should catch regressions.
  • Loading branch information
@kpschoedel
kpschoedel authored Jun 21, 2021
1 parent 7ddf8be commit f9550e1
Show file tree
Hide file tree
Showing 56 changed files with 1,334 additions and 2,116 deletions.
4 changes: 1 addition & 3 deletions src/app/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ CHIP_ERROR Command::ProcessCommandMessage(System::PacketBufferHandle && payload,

void Command::Shutdown()
{
VerifyOrExit(mState != CommandState::Uninitialized, );
VerifyOrReturn(mState != CommandState::Uninitialized);
mCommandMessageWriter.Reset();

AbortExistingExchangeContext();
Expand All @@ -136,8 +136,6 @@ void Command::Shutdown()
ClearState();

mCommandIndex = 0;
exit:
return;
}

CHIP_ERROR Command::PrepareCommand(const CommandPathParams & aCommandPathParams, bool aIsStatus)
Expand Down
90 changes: 38 additions & 52 deletions src/app/EventManagement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,29 +668,30 @@ CHIP_ERROR EventManagement::EventIterator(const TLVReader & aReader, size_t aDep

CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aDepth, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
TLV::TLVWriter checkpoint;
EventLoadOutContext * loadOutContext = static_cast<EventLoadOutContext *>(apContext);
err = EventIterator(aReader, aDepth, loadOutContext);
EventLoadOutContext * const loadOutContext = static_cast<EventLoadOutContext *>(apContext);
CHIP_ERROR err = EventIterator(aReader, aDepth, loadOutContext);
loadOutContext->mCurrentEventNumber++;
if (err == CHIP_EVENT_ID_FOUND)
{
// checkpoint the writer
checkpoint = loadOutContext->mWriter;
TLV::TLVWriter checkpoint = loadOutContext->mWriter;

err = CopyEvent(aReader, loadOutContext->mWriter, loadOutContext);

// CHIP_NO_ERROR and CHIP_END_OF_TLV signify a
// successful copy. In all other cases, roll back the
// writer state back to the checkpoint, i.e., the state
// before we began the copy operation.
VerifyOrExit((err == CHIP_NO_ERROR) || (err == CHIP_END_OF_TLV), loadOutContext->mWriter = checkpoint);
if ((err != CHIP_NO_ERROR) && (err != CHIP_END_OF_TLV))
{
loadOutContext->mWriter = checkpoint;
return err;
}

loadOutContext->mPreviousSystemTime.mValue = loadOutContext->mCurrentSystemTime.mValue;
loadOutContext->mFirst = false;
}

exit:
return err;
}

Expand Down Expand Up @@ -734,86 +735,75 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo *

CHIP_ERROR EventManagement::GetEventReader(TLVReader & aReader, PriorityLevel aPriority, CircularEventBufferWrapper * apBufWrapper)
{
CHIP_ERROR err = CHIP_NO_ERROR;
CircularEventReader reader;
CircularEventBuffer * buffer = GetPriorityBuffer(aPriority);
VerifyOrExit(buffer != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(buffer != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
apBufWrapper->mpCurrent = buffer;

CircularEventReader reader;
reader.Init(apBufWrapper);
aReader.Init(reader);
exit:
return err;

return CHIP_NO_ERROR;
}

CHIP_ERROR EventManagement::FetchEventParameters(const TLVReader & aReader, size_t aDepth, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
EventEnvelopeContext * envelope = static_cast<EventEnvelopeContext *>(apContext);
EventEnvelopeContext * const envelope = static_cast<EventEnvelopeContext *>(apContext);
TLVReader reader;
uint16_t extPriority; // Note: the type here matches the type case in EventManagement::LogEvent, priority section
reader.Init(aReader);

if (reader.GetTag() == TLV::ContextTag(EventDataElement::kCsTag_EventPath))
{
EventPath::Parser path;
SuccessOrExit(err = path.Init(aReader));
SuccessOrExit(err = path.GetNodeId(&(envelope->mNodeId)));
SuccessOrExit(err = path.GetEndpointId(&(envelope->mEndpointId)));
SuccessOrExit(err = path.GetClusterId(&(envelope->mClusterId)));
SuccessOrExit(err = path.GetEventId(&(envelope->mEventId)));
ReturnErrorOnFailure(path.Init(aReader));
ReturnErrorOnFailure(path.GetNodeId(&(envelope->mNodeId)));
ReturnErrorOnFailure(path.GetEndpointId(&(envelope->mEndpointId)));
ReturnErrorOnFailure(path.GetClusterId(&(envelope->mClusterId)));
ReturnErrorOnFailure(path.GetEventId(&(envelope->mEventId)));
envelope->mFieldsToRead |= 1 << EventDataElement::kCsTag_EventPath;
}

if (reader.GetTag() == TLV::ContextTag(EventDataElement::kCsTag_PriorityLevel))
{
err = reader.Get(extPriority);
SuccessOrExit(err);
uint16_t extPriority; // Note: the type here matches the type case in EventManagement::LogEvent, priority section
ReturnErrorOnFailure(reader.Get(extPriority));
envelope->mPriority = static_cast<PriorityLevel>(extPriority);
envelope->mFieldsToRead |= 1 << EventDataElement::kCsTag_PriorityLevel;
}

if (reader.GetTag() == TLV::ContextTag(EventDataElement::kCsTag_DeltaSystemTimestamp))
{
err = reader.Get(envelope->mDeltaSystemTime.mValue);
SuccessOrExit(err);
ReturnErrorOnFailure(reader.Get(envelope->mDeltaSystemTime.mValue));

envelope->mFieldsToRead |= 1 << EventDataElement::kCsTag_DeltaSystemTimestamp;
}

exit:
return err;
return CHIP_NO_ERROR;
}

CHIP_ERROR EventManagement::EvictEvent(CHIPCircularTLVBuffer & apBuffer, void * apAppData, TLVReader & aReader)
{
ReclaimEventCtx * ctx = static_cast<ReclaimEventCtx *>(apAppData);
CircularEventBuffer * eventBuffer = ctx->mpEventBuffer;
TLVType containerType;
EventEnvelopeContext context;
const bool recurse = false;
CHIP_ERROR err;
PriorityLevel imp = PriorityLevel::Invalid;

// pull out the delta time, pull out the priority
err = aReader.Next();
SuccessOrExit(err);
ReturnErrorOnFailure(aReader.Next());

err = aReader.EnterContainer(containerType);
SuccessOrExit(err);
TLVType containerType;
ReturnErrorOnFailure(aReader.EnterContainer(containerType));

err = TLV::Utilities::Iterate(aReader, FetchEventParameters, &context, recurse);
EventEnvelopeContext context;
constexpr bool recurse = false;
CHIP_ERROR err = TLV::Utilities::Iterate(aReader, FetchEventParameters, &context, recurse);
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);

err = aReader.ExitContainer(containerType);
ReturnErrorOnFailure(err);

SuccessOrExit(err);
ReturnErrorOnFailure(aReader.ExitContainer(containerType));

imp = static_cast<PriorityLevel>(context.mPriority);
const PriorityLevel imp = static_cast<PriorityLevel>(context.mPriority);

ReclaimEventCtx * const ctx = static_cast<ReclaimEventCtx *>(apAppData);
CircularEventBuffer * const eventBuffer = ctx->mpEventBuffer;
if (eventBuffer->IsFinalDestinationForPriority(imp))
{
// event is getting dropped. Increase the event number and first timestamp.
Expand All @@ -826,16 +816,12 @@ CHIP_ERROR EventManagement::EvictEvent(CHIPCircularTLVBuffer & apBuffer, void *
" };",
static_cast<unsigned>(eventBuffer->GetPriorityLevel()), static_cast<unsigned>(imp), ChipLogValueX64(numEventsToDrop));
ctx->mSpaceNeededForMovedEvent = 0;
}
else
{
// event is not getting dropped. Note how much space it requires, and return.
ctx->mSpaceNeededForMovedEvent = aReader.GetLengthRead();
err = CHIP_END_OF_TLV;
return CHIP_NO_ERROR;
}

exit:
return err;
// event is not getting dropped. Note how much space it requires, and return.
ctx->mSpaceNeededForMovedEvent = aReader.GetLengthRead();
return CHIP_END_OF_TLV;
}

CHIP_ERROR EventManagement::ScheduleFlushIfNeeded(EventOptions::Type aUrgent)
Expand Down
23 changes: 7 additions & 16 deletions src/app/InteractionModelEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,12 @@ InteractionModelEngine * InteractionModelEngine::GetInstance()

CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate)
{
CHIP_ERROR err = CHIP_NO_ERROR;

mpExchangeMgr = apExchangeMgr;
mpDelegate = apDelegate;

err = mpExchangeMgr->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id, this);
SuccessOrExit(err);
ReturnErrorOnFailure(mpExchangeMgr->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id, this));

mReportingEngine.Init();
SuccessOrExit(err);

for (uint32_t index = 0; index < IM_SERVER_MAX_NUM_PATH_GROUPS - 1; index++)
{
Expand All @@ -60,8 +56,7 @@ CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeM
mClusterInfoPool[IM_SERVER_MAX_NUM_PATH_GROUPS - 1].mpNext = nullptr;
mpNextAvailableClusterInfo = mClusterInfoPool;

exit:
return err;
return CHIP_NO_ERROR;
}

void InteractionModelEngine::Shutdown()
Expand Down Expand Up @@ -125,26 +120,22 @@ void InteractionModelEngine::Shutdown()

CHIP_ERROR InteractionModelEngine::NewCommandSender(CommandSender ** const apCommandSender)
{
CHIP_ERROR err = CHIP_ERROR_NO_MEMORY;
*apCommandSender = nullptr;

for (auto & commandSender : mCommandSenderObjs)
{
if (commandSender.IsFree())
{
*apCommandSender = &commandSender;
err = commandSender.Init(mpExchangeMgr, mpDelegate);
if (CHIP_NO_ERROR != err)
const CHIP_ERROR err = commandSender.Init(mpExchangeMgr, mpDelegate);
if (err == CHIP_NO_ERROR)
{
*apCommandSender = nullptr;
ExitNow();
*apCommandSender = &commandSender;
}
break;
return err;
}
}

exit:
return err;
return CHIP_ERROR_NO_MEMORY;
}

CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClient)
Expand Down
17 changes: 6 additions & 11 deletions src/app/MessageDef/AttributeDataElement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,27 +393,22 @@ AttributePath::Builder & AttributeDataElement::Builder::CreateAttributePathBuild
AttributeDataElement::Builder & AttributeDataElement::Builder::DataVersion(const chip::DataVersion aDataVersion)
{
// skip if error has already been set
SuccessOrExit(mError);

mError = mpWriter->Put(chip::TLV::ContextTag(kCsTag_DataVersion), aDataVersion);
ChipLogFunctError(mError);

exit:
if (mError == CHIP_NO_ERROR)
{
mError = mpWriter->Put(chip::TLV::ContextTag(kCsTag_DataVersion), aDataVersion);
ChipLogFunctError(mError);
}
return *this;
}

AttributeDataElement::Builder & AttributeDataElement::Builder::MoreClusterData(const bool aMoreClusterData)
{
// skip if error has already been set
SuccessOrExit(mError);

if (aMoreClusterData)
if ((mError == CHIP_NO_ERROR) && aMoreClusterData)
{
mError = mpWriter->PutBoolean(chip::TLV::ContextTag(kCsTag_MoreClusterDataFlag), true);
ChipLogFunctError(mError);
}

exit:
return *this;
}

Expand Down
22 changes: 10 additions & 12 deletions src/app/MessageDef/AttributeDataVersionList.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,22 @@ CHIP_ERROR AttributeDataVersionList::Parser::GetVersion(chip::DataVersion * cons
AttributeDataVersionList::Builder & AttributeDataVersionList::Builder::AddVersion(const uint64_t aVersion)
{
// skip if error has already been set
SuccessOrExit(mError);

mError = mpWriter->Put(chip::TLV::AnonymousTag, aVersion);
ChipLogFunctError(mError);

exit:
if (mError == CHIP_NO_ERROR)
{
mError = mpWriter->Put(chip::TLV::AnonymousTag, aVersion);
ChipLogFunctError(mError);
}
return *this;
}

AttributeDataVersionList::Builder & AttributeDataVersionList::Builder::AddNull(void)
{
// skip if error has already been set
SuccessOrExit(mError);

mError = mpWriter->PutNull(chip::TLV::AnonymousTag);
ChipLogFunctError(mError);

exit:
if (mError == CHIP_NO_ERROR)
{
mError = mpWriter->PutNull(chip::TLV::AnonymousTag);
ChipLogFunctError(mError);
}
return *this;
}

Expand Down
Loading

0 comments on commit f9550e1

Please sign in to comment.