Skip to content

Commit 5638457

Browse files
vivien-applepull[bot]
vivien-apple
authored and
pull[bot]
committed
[chip-tool] Some CHIP_ERROR returned values are ignored (#25486)
1 parent fd9c815 commit 5638457

File tree

11 files changed

+83
-64
lines changed

11 files changed

+83
-64
lines changed

‎examples/chip-tool/commands/clusters/CustomArgument.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ class CustomArgument
279279
{
280280
chip::TLV::TLVReader reader;
281281
reader.Init(mData, mDataLen);
282-
reader.Next();
282+
ReturnErrorOnFailure(reader.Next());
283283

284284
return writer.CopyElement(tag, reader);
285285
}

‎examples/chip-tool/commands/clusters/DataModelLogger.h

+7-16
Original file line numberDiff line numberDiff line change
@@ -103,26 +103,20 @@ class DataModelLogger
103103
template <typename X, typename std::enable_if_t<std::is_enum<X>::value, int> = 0>
104104
static CHIP_ERROR LogValue(const char * label, size_t indent, X value)
105105
{
106-
DataModelLogger::LogValue(label, indent, chip::to_underlying(value));
107-
return CHIP_NO_ERROR;
106+
return DataModelLogger::LogValue(label, indent, chip::to_underlying(value));
108107
}
109108

110109
template <typename X>
111110
static CHIP_ERROR LogValue(const char * label, size_t indent, chip::BitFlags<X> value)
112111
{
113-
DataModelLogger::LogValue(label, indent, value.Raw());
114-
return CHIP_NO_ERROR;
112+
return DataModelLogger::LogValue(label, indent, value.Raw());
115113
}
116114

117115
template <typename T>
118116
static CHIP_ERROR LogValue(const char * label, size_t indent, const chip::app::DataModel::DecodableList<T> & value)
119117
{
120-
size_t count = 0;
121-
CHIP_ERROR err = value.ComputeSize(&count);
122-
if (err != CHIP_NO_ERROR)
123-
{
124-
return err;
125-
}
118+
size_t count = 0;
119+
ReturnErrorOnFailure(value.ComputeSize(&count));
126120
DataModelLogger::LogString(label, indent, std::to_string(count) + " entries");
127121

128122
auto iter = value.begin();
@@ -146,21 +140,18 @@ class DataModelLogger
146140
if (value.IsNull())
147141
{
148142
DataModelLogger::LogString(label, indent, "null");
149-
}
150-
else
151-
{
152-
DataModelLogger::LogValue(label, indent, value.Value());
143+
return CHIP_NO_ERROR;
153144
}
154145

155-
return CHIP_NO_ERROR;
146+
return DataModelLogger::LogValue(label, indent, value.Value());
156147
}
157148

158149
template <typename T>
159150
static CHIP_ERROR LogValue(const char * label, size_t indent, const chip::Optional<T> & value)
160151
{
161152
if (value.HasValue())
162153
{
163-
DataModelLogger::LogValue(label, indent, value.Value());
154+
return DataModelLogger::LogValue(label, indent, value.Value());
164155
}
165156

166157
return CHIP_NO_ERROR;

‎examples/chip-tool/commands/common/CHIPCommand.cpp

+10-1
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,16 @@ CHIP_ERROR CHIPCommand::StartWaiting(chip::System::Clock::Timeout duration)
506506
mWaitingForResponse = true;
507507
}
508508

509-
chip::DeviceLayer::PlatformMgr().ScheduleWork(RunQueuedCommand, reinterpret_cast<intptr_t>(this));
509+
auto err = chip::DeviceLayer::PlatformMgr().ScheduleWork(RunQueuedCommand, reinterpret_cast<intptr_t>(this));
510+
if (CHIP_NO_ERROR != err)
511+
{
512+
{
513+
std::lock_guard<std::mutex> lk(cvWaitingForResponseMutex);
514+
mWaitingForResponse = false;
515+
}
516+
return err;
517+
}
518+
510519
auto waitingUntil = std::chrono::system_clock::now() + std::chrono::duration_cast<std::chrono::seconds>(duration);
511520
{
512521
std::unique_lock<std::mutex> lk(cvWaitingForResponseMutex);

‎examples/chip-tool/commands/common/RemoteDataModelLogger.cpp

+27-24
Original file line numberDiff line numberDiff line change
@@ -169,48 +169,51 @@ CHIP_ERROR LogDiscoveredNodeData(const chip::Dnssd::DiscoveredNodeData & nodeDat
169169
{
170170
VerifyOrReturnError(gDelegate != nullptr, CHIP_NO_ERROR);
171171

172-
if (!chip::CanCastTo<uint8_t>(nodeData.resolutionData.numIPs))
172+
auto & resolutionData = nodeData.resolutionData;
173+
auto & commissionData = nodeData.commissionData;
174+
175+
if (!chip::CanCastTo<uint8_t>(resolutionData.numIPs))
173176
{
174177
ChipLogError(chipTool, "Too many ips.");
175178
return CHIP_ERROR_INVALID_ARGUMENT;
176179
}
177180

178-
if (!chip::CanCastTo<uint64_t>(nodeData.commissionData.rotatingIdLen))
181+
if (!chip::CanCastTo<uint64_t>(commissionData.rotatingIdLen))
179182
{
180183
ChipLogError(chipTool, "Can not convert rotatingId to json format.");
181184
return CHIP_ERROR_INVALID_ARGUMENT;
182185
}
183186

184187
char rotatingId[chip::Dnssd::kMaxRotatingIdLen * 2 + 1] = "";
185-
chip::Encoding::BytesToUppercaseHexString(nodeData.commissionData.rotatingId, nodeData.commissionData.rotatingIdLen, rotatingId,
186-
sizeof(rotatingId));
188+
ReturnErrorOnFailure(chip::Encoding::BytesToUppercaseHexString(commissionData.rotatingId, commissionData.rotatingIdLen,
189+
rotatingId, sizeof(rotatingId)));
187190

188191
Json::Value value;
189-
value["hostName"] = nodeData.resolutionData.hostName;
190-
value["instanceName"] = nodeData.commissionData.instanceName;
191-
value["longDiscriminator"] = nodeData.commissionData.longDiscriminator;
192-
value["shortDiscriminator"] = ((nodeData.commissionData.longDiscriminator >> 8) & 0x0F);
193-
value["vendorId"] = nodeData.commissionData.vendorId;
194-
value["productId"] = nodeData.commissionData.productId;
195-
value["commissioningMode"] = nodeData.commissionData.commissioningMode;
196-
value["deviceType"] = nodeData.commissionData.deviceType;
197-
value["deviceName"] = nodeData.commissionData.deviceName;
192+
value["hostName"] = resolutionData.hostName;
193+
value["instanceName"] = commissionData.instanceName;
194+
value["longDiscriminator"] = commissionData.longDiscriminator;
195+
value["shortDiscriminator"] = ((commissionData.longDiscriminator >> 8) & 0x0F);
196+
value["vendorId"] = commissionData.vendorId;
197+
value["productId"] = commissionData.productId;
198+
value["commissioningMode"] = commissionData.commissioningMode;
199+
value["deviceType"] = commissionData.deviceType;
200+
value["deviceName"] = commissionData.deviceName;
198201
value["rotatingId"] = rotatingId;
199-
value["rotatingIdLen"] = static_cast<uint64_t>(nodeData.commissionData.rotatingIdLen);
200-
value["pairingHint"] = nodeData.commissionData.pairingHint;
201-
value["pairingInstruction"] = nodeData.commissionData.pairingInstruction;
202-
value["supportsTcp"] = nodeData.resolutionData.supportsTcp;
203-
value["port"] = nodeData.resolutionData.port;
204-
value["numIPs"] = static_cast<uint8_t>(nodeData.resolutionData.numIPs);
205-
206-
if (nodeData.resolutionData.mrpRetryIntervalIdle.HasValue())
202+
value["rotatingIdLen"] = static_cast<uint64_t>(commissionData.rotatingIdLen);
203+
value["pairingHint"] = commissionData.pairingHint;
204+
value["pairingInstruction"] = commissionData.pairingInstruction;
205+
value["supportsTcp"] = resolutionData.supportsTcp;
206+
value["port"] = resolutionData.port;
207+
value["numIPs"] = static_cast<uint8_t>(resolutionData.numIPs);
208+
209+
if (resolutionData.mrpRetryIntervalIdle.HasValue())
207210
{
208-
value["mrpRetryIntervalIdle"] = nodeData.resolutionData.mrpRetryIntervalIdle.Value().count();
211+
value["mrpRetryIntervalIdle"] = resolutionData.mrpRetryIntervalIdle.Value().count();
209212
}
210213

211-
if (nodeData.resolutionData.mrpRetryIntervalActive.HasValue())
214+
if (resolutionData.mrpRetryIntervalActive.HasValue())
212215
{
213-
value["mrpRetryIntervalActive"] = nodeData.resolutionData.mrpRetryIntervalActive.Value().count();
216+
value["mrpRetryIntervalActive"] = resolutionData.mrpRetryIntervalActive.Value().count();
214217
}
215218

216219
Json::Value rootValue;

‎examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ void DiscoverCommissionablesCommandBase::OnDiscoveredDevice(const chip::Dnssd::D
3030
if (mDiscoverOnce.ValueOr(true))
3131
{
3232
mCommissioner->RegisterDeviceDiscoveryDelegate(nullptr);
33-
mCommissioner->StopCommissionableDiscovery();
34-
SetCommandExitStatus(CHIP_NO_ERROR);
33+
auto err = mCommissioner->StopCommissionableDiscovery();
34+
SetCommandExitStatus(err);
3535
}
3636
}
3737

‎examples/chip-tool/commands/interactive/InteractiveCommands.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,10 @@ bool InteractiveCommand::ParseCommand(char * command, int * status)
306306
{
307307
if (strcmp(command, kInteractiveModeStopCommand) == 0)
308308
{
309-
chip::DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredCleanups, 0);
309+
// If scheduling the cleanup fails, there is not much we can do.
310+
// But if something went wrong while the application is leaving it could be because things have
311+
// not been cleaned up properly, so it is still useful to log the failure.
312+
LogErrorOnFailure(chip::DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredCleanups, 0));
310313
return false;
311314
}
312315

‎examples/chip-tool/commands/pairing/PairingCommand.cpp

+24-10
Original file line numberDiff line numberDiff line change
@@ -269,22 +269,32 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
269269

270270
void PairingCommand::OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData)
271271
{
272-
// Ignore nodes with closed comissioning window
272+
// Ignore nodes with closed commissioning window
273273
VerifyOrReturn(nodeData.commissionData.commissioningMode != 0);
274274

275-
const uint16_t port = nodeData.resolutionData.port;
275+
auto & resolutionData = nodeData.resolutionData;
276+
277+
const uint16_t port = resolutionData.port;
276278
char buf[chip::Inet::IPAddress::kMaxStringLength];
277-
nodeData.resolutionData.ipAddress[0].ToString(buf);
279+
resolutionData.ipAddress[0].ToString(buf);
278280
ChipLogProgress(chipTool, "Discovered Device: %s:%u", buf, port);
279281

280282
// Stop Mdns discovery.
281-
CurrentCommissioner().StopCommissionableDiscovery();
283+
auto err = CurrentCommissioner().StopCommissionableDiscovery();
284+
285+
// Some platforms does not implement a mechanism to stop mdns browse, so
286+
// we just ignore CHIP_ERROR_NOT_IMPLEMENTED instead of bailing out.
287+
if (CHIP_NO_ERROR != err && CHIP_ERROR_NOT_IMPLEMENTED != err)
288+
{
289+
SetCommandExitStatus(err);
290+
return;
291+
}
292+
282293
CurrentCommissioner().RegisterDeviceDiscoveryDelegate(nullptr);
283294

284-
Inet::InterfaceId interfaceId =
285-
nodeData.resolutionData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.resolutionData.interfaceId : Inet::InterfaceId::Null();
286-
PeerAddress peerAddress = PeerAddress::UDP(nodeData.resolutionData.ipAddress[0], port, interfaceId);
287-
CHIP_ERROR err = Pair(mNodeId, peerAddress);
295+
auto interfaceId = resolutionData.ipAddress[0].IsIPv6LinkLocal() ? resolutionData.interfaceId : Inet::InterfaceId::Null();
296+
auto peerAddress = PeerAddress::UDP(resolutionData.ipAddress[0], port, interfaceId);
297+
err = Pair(mNodeId, peerAddress);
288298
if (CHIP_NO_ERROR != err)
289299
{
290300
SetCommandExitStatus(err);
@@ -320,6 +330,10 @@ void PairingCommand::OnDeviceAttestationCompleted(chip::Controller::DeviceCommis
320330
chip::Credentials::AttestationVerificationResult attestationResult)
321331
{
322332
// Bypass attestation verification, continue with success
323-
deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device,
324-
chip::Credentials::AttestationVerificationResult::kSuccess);
333+
auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
334+
device, chip::Credentials::AttestationVerificationResult::kSuccess);
335+
if (CHIP_NO_ERROR != err)
336+
{
337+
SetCommandExitStatus(err);
338+
}
325339
}

‎examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,12 @@ CHIP_ERROR SetupPayloadGenerateQRCodeCommand::PopulatePayloadTLVFromBytes(SetupP
118118
{
119119
// First clear out all the existing TVL bits from the payload. Ignore
120120
// errors here, because we don't care if those bits are not present.
121-
payload.removeSerialNumber();
121+
(void) payload.removeSerialNumber();
122122

123123
auto existingVendorData = payload.getAllOptionalVendorData();
124124
for (auto & data : existingVendorData)
125125
{
126-
payload.removeOptionalVendorData(data.tag);
126+
(void) payload.removeOptionalVendorData(data.tag);
127127
}
128128

129129
if (tlvBytes.empty())

‎examples/common/websocket-server/WebSocketServer.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,8 @@ bool WebSocketServer::OnWebSocketMessageReceived(char * msg)
200200
return shouldContinue;
201201
}
202202

203-
CHIP_ERROR WebSocketServer::Send(const char * msg)
203+
void WebSocketServer::Send(const char * msg)
204204
{
205205
std::lock_guard<std::mutex> lock(gMutex);
206206
gMessageQueue.push_back(msg);
207-
return CHIP_NO_ERROR;
208207
}

‎examples/common/websocket-server/WebSocketServer.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class WebSocketServer : public WebSocketServerDelegate
2929
{
3030
public:
3131
CHIP_ERROR Run(chip::Optional<uint16_t> port, WebSocketServerDelegate * delegate);
32-
CHIP_ERROR Send(const char * msg);
32+
void Send(const char * msg);
3333

3434
bool OnWebSocketMessageReceived(char * msg) override;
3535

‎examples/placeholder/linux/InteractiveServer.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ bool InteractiveServer::Command(const chip::app::ConcreteCommandPath & path)
226226

227227
auto valueStr = JsonToString(value);
228228
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
229-
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
229+
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
230230
gInteractiveServerResult.Reset();
231231
return mIsReady;
232232
}
@@ -243,7 +243,7 @@ bool InteractiveServer::ReadAttribute(const chip::app::ConcreteAttributePath & p
243243

244244
auto valueStr = JsonToString(value);
245245
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
246-
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
246+
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
247247
gInteractiveServerResult.Reset();
248248
return mIsReady;
249249
}
@@ -260,7 +260,7 @@ bool InteractiveServer::WriteAttribute(const chip::app::ConcreteAttributePath &
260260

261261
auto valueStr = JsonToString(value);
262262
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
263-
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
263+
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
264264
gInteractiveServerResult.Reset();
265265
return mIsReady;
266266
}
@@ -273,6 +273,6 @@ void InteractiveServer::CommissioningComplete()
273273
Json::Value value = Json::objectValue;
274274
auto valueStr = JsonToString(value);
275275
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
276-
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
276+
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
277277
gInteractiveServerResult.Reset();
278278
}

0 commit comments

Comments
 (0)