From 127fc5aea90c236d2bb1463e2d47ef49ee319245 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jul 2023 11:38:09 -0400 Subject: [PATCH 01/13] Make TraceDecoder not stateful, log inbound/outbount --- examples/common/tracing/TraceDecoder.cpp | 103 ++++++++-------------- examples/common/tracing/TraceDecoder.h | 8 +- examples/common/tracing/TraceHandlers.cpp | 6 +- src/transport/Session.h | 1 - 4 files changed, 44 insertions(+), 74 deletions(-) diff --git a/examples/common/tracing/TraceDecoder.cpp b/examples/common/tracing/TraceDecoder.cpp index 2bb3c5ee213ce0..ed0b8e375f46cc 100644 --- a/examples/common/tracing/TraceDecoder.cpp +++ b/examples/common/tracing/TraceDecoder.cpp @@ -50,11 +50,34 @@ constexpr const char * kPayloadSizeKey = "payload_size"; constexpr const char * kPayloadEncryptedDataKey = "payload_hex_encrypted"; constexpr const char * kPayloadEncryptedSizeKey = "payload_size_encrypted"; constexpr const char * kPayloadEncryptedBufferPtrKey = "buffer_ptr"; -constexpr const char * kSourceKey = "source"; -constexpr const char * kDestinationKey = "destination"; namespace chip { namespace trace { +namespace { + +constexpr const char * kDirectionKey = "direction"; + +bool IsOutbound(const Json::Value & json) +{ + if (!json.isMember(kDirectionKey)) + { + return false; // unknown + } + + return strcmp(json[kDirectionKey].asCString(), "outbound") == 0; +} + +bool IsInbound(const Json::Value & json) +{ + if (!json.isMember(kDirectionKey)) + { + return false; // unknown + } + + return strcmp(json[kDirectionKey].asCString(), "inbound") == 0; +} + +} // namespace using namespace logging; @@ -83,45 +106,8 @@ CHIP_ERROR TraceDecoder::ReadString(const char * str) str += strlen(jsonPrefix); Json::Reader reader; - - if (mJsonBuffer.empty()) - { - VerifyOrReturnError(reader.parse(str, mJsonBuffer), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(mJsonBuffer.isMember(kPayloadDataKey) && mJsonBuffer.isMember(kPayloadSizeKey), - CHIP_ERROR_INCORRECT_STATE); - return CHIP_NO_ERROR; - } - Json::Value json; VerifyOrReturnError(reader.parse(str, json), CHIP_ERROR_INVALID_ARGUMENT); - - // If there is a source, then it means the previously saved payload is an encrypted to decode, otherwise - // the previously saved payload is the non encrypted version, and the current decoded one is the encrypted version. - if (mJsonBuffer.isMember(kSourceKey)) - { - json[kPayloadEncryptedDataKey] = mJsonBuffer[kPayloadDataKey]; - json[kPayloadEncryptedSizeKey] = mJsonBuffer[kPayloadSizeKey]; - } - else - { - auto data = json[kPayloadDataKey]; - auto size = json[kPayloadSizeKey]; - json[kPayloadDataKey] = mJsonBuffer[kPayloadDataKey]; - json[kPayloadSizeKey] = mJsonBuffer[kPayloadSizeKey]; - json[kPayloadEncryptedDataKey] = data; - json[kPayloadEncryptedSizeKey] = size; - } - mJsonBuffer.removeMember(kPayloadDataKey); - mJsonBuffer.removeMember(kPayloadSizeKey); - - // If there is additional data in the previously saved json copy all of it. - for (const auto & key : mJsonBuffer.getMemberNames()) - { - json[key] = mJsonBuffer[key]; - mJsonBuffer.removeMember(key); - } - - VerifyOrReturnError(json.isMember(kSourceKey) || json.isMember(kDestinationKey), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(json.isMember(kProtocolIdKey), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(json.isMember(kProtocolCodeKey), CHIP_ERROR_INCORRECT_STATE); @@ -138,20 +124,19 @@ CHIP_ERROR TraceDecoder::LogJSON(Json::Value & json) return CHIP_NO_ERROR; } - if (!mOptions.mEnableMessageInitiator && json.isMember(kDestinationKey)) + if (!mOptions.mEnableMessageInitiator && IsOutbound(json)) { return CHIP_NO_ERROR; } - if (!mOptions.mEnableMessageResponder && json.isMember(kSourceKey)) + if (!mOptions.mEnableMessageResponder && IsInbound(json)) { return CHIP_NO_ERROR; } - bool isResponse = json.isMember(kSourceKey) ? true : false; + bool isResponse = IsInbound(json); ReturnErrorOnFailure(LogAndConsumeProtocol(json)); ReturnErrorOnFailure(MaybeLogAndConsumeHeaderFlags(json)); - ReturnErrorOnFailure(MaybeLogAndConsumeEncryptedPayload(json)); ReturnErrorOnFailure(MaybeLogAndConsumePayload(json, isResponse)); ReturnErrorOnFailure(MaybeLogAndConsumeOthers(json)); @@ -168,26 +153,6 @@ CHIP_ERROR TraceDecoder::MaybeLogAndConsumeHeaderFlags(Json::Value & json) return CHIP_NO_ERROR; } -CHIP_ERROR TraceDecoder::MaybeLogAndConsumeEncryptedPayload(Json::Value & json) -{ - if (mOptions.mEnableDataEncryptedPayload) - { - size_t size = static_cast(json[kPayloadEncryptedSizeKey].asLargestUInt()); - if (size) - { - auto payload = json[kPayloadEncryptedDataKey].asString(); - auto bufferPtr = json[kPayloadEncryptedBufferPtrKey].asString(); - auto scopedIndent = ScopedLogIndentWithSize("Encrypted Payload", size); - Log("data", payload.c_str()); - Log("buffer_ptr", bufferPtr.c_str()); - } - } - json.removeMember(kPayloadEncryptedSizeKey); - json.removeMember(kPayloadEncryptedDataKey); - json.removeMember(kPayloadEncryptedBufferPtrKey); - return CHIP_NO_ERROR; -} - CHIP_ERROR TraceDecoder::MaybeLogAndConsumeOthers(Json::Value & json) { std::vector keys = json.getMemberNames(); @@ -218,8 +183,13 @@ CHIP_ERROR TraceDecoder::LogAndConsumeProtocol(Json::Value & json) chip::StringBuilderBase builder(protocolInfo, sizeof(protocolInfo)); - builder.Add(json.isMember(kSourceKey) ? "<< from " : ">> to "); - builder.Add(json.isMember(kSourceKey) ? json[kSourceKey].asCString() : json[kDestinationKey].asCString()); + builder.Add(IsInbound(json) ? "<< from " : ">> to "); + + if (json.isMember("peer")) { + builder.Add(json["peer"].asCString()); + } else { + builder.Add("UNKNOWN"); + } builder.Add(" "); auto msgCounter = static_cast(json[kMessageCounterKey].asLargestUInt()); @@ -258,11 +228,10 @@ CHIP_ERROR TraceDecoder::LogAndConsumeProtocol(Json::Value & json) ChipLogProgress(DataManagement, "%s", builder.c_str()); - json.removeMember(kSourceKey); - json.removeMember(kDestinationKey); json.removeMember(kSessionIdKey); json.removeMember(kExchangeIdKey); json.removeMember(kMessageCounterKey); + json.removeMember(kDirectionKey); return CHIP_NO_ERROR; } diff --git a/examples/common/tracing/TraceDecoder.h b/examples/common/tracing/TraceDecoder.h index cc8dbd2374f9d5..fd5f3d9e551fdb 100644 --- a/examples/common/tracing/TraceDecoder.h +++ b/examples/common/tracing/TraceDecoder.h @@ -42,7 +42,11 @@ class TraceDecoder : public TraceStream { char buffer[2048] = {}; snprintf(buffer, sizeof(buffer), " %s\t %s", tag.c_str(), data.c_str()); - ReadString(buffer); + CHIP_ERROR err = ReadString(buffer); + + if (err != CHIP_NO_ERROR) { + ChipLogError(Automation, "Failed to add field: %" CHIP_ERROR_FORMAT, err.Format()); + } } void FinishEvent() override {} @@ -55,12 +59,10 @@ class TraceDecoder : public TraceStream CHIP_ERROR MaybeLogAndConsumeMessageFlags(Json::Value & value); CHIP_ERROR MaybeLogAndConsumeExchangeFlags(Json::Value & value); CHIP_ERROR MaybeLogAndConsumePayload(Json::Value & value, bool isResponse); - CHIP_ERROR MaybeLogAndConsumeEncryptedPayload(Json::Value & value); CHIP_ERROR MaybeLogAndConsumeOthers(Json::Value & value); private: TraceDecoderOptions mOptions; - Json::Value mJsonBuffer; }; } // namespace trace diff --git a/examples/common/tracing/TraceHandlers.cpp b/examples/common/tracing/TraceHandlers.cpp index d595580fa7619a..0a3e5591e5935d 100644 --- a/examples/common/tracing/TraceHandlers.cpp +++ b/examples/common/tracing/TraceHandlers.cpp @@ -233,7 +233,7 @@ void SecureMessageSentHandler(const TraceSecureMessageSentData * eventData) return; } - std::string jsonBody = "{"; + std::string jsonBody = "{ \"direction\": \"outbound\", "; jsonBody += PacketHeaderToJson(eventData->packetHeader); jsonBody += ", "; jsonBody += PayloadHeaderToJson(eventData->payloadHeader); @@ -255,8 +255,8 @@ void SecureMessageReceivedHandler(const TraceSecureMessageReceivedData * eventDa return; } - std::string jsonBody = "{"; - jsonBody += AsFirstJsonKey("peer_address", AsJsonString(eventData->peerAddress)); + std::string jsonBody = "{ \"direction\": \"inbound\", "; + jsonBody += AsFirstJsonKey("peer", AsJsonString(eventData->peerAddress)); jsonBody += ", "; jsonBody += PacketHeaderToJson(eventData->packetHeader); jsonBody += ", "; diff --git a/src/transport/Session.h b/src/transport/Session.h index 29ebf593a9b5e3..14139b56df9c00 100644 --- a/src/transport/Session.h +++ b/src/transport/Session.h @@ -27,7 +27,6 @@ #include #include #include -#include namespace chip { namespace Transport { From 33b1629d766d34b5456048bc3dfd67cafc3d3683 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jul 2023 11:38:22 -0400 Subject: [PATCH 02/13] Restyle --- examples/common/tracing/TraceDecoder.cpp | 7 +++++-- examples/common/tracing/TraceDecoder.h | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/examples/common/tracing/TraceDecoder.cpp b/examples/common/tracing/TraceDecoder.cpp index ed0b8e375f46cc..9dc35168bdc576 100644 --- a/examples/common/tracing/TraceDecoder.cpp +++ b/examples/common/tracing/TraceDecoder.cpp @@ -185,9 +185,12 @@ CHIP_ERROR TraceDecoder::LogAndConsumeProtocol(Json::Value & json) builder.Add(IsInbound(json) ? "<< from " : ">> to "); - if (json.isMember("peer")) { + if (json.isMember("peer")) + { builder.Add(json["peer"].asCString()); - } else { + } + else + { builder.Add("UNKNOWN"); } diff --git a/examples/common/tracing/TraceDecoder.h b/examples/common/tracing/TraceDecoder.h index fd5f3d9e551fdb..4e70f5f12f1535 100644 --- a/examples/common/tracing/TraceDecoder.h +++ b/examples/common/tracing/TraceDecoder.h @@ -44,7 +44,8 @@ class TraceDecoder : public TraceStream snprintf(buffer, sizeof(buffer), " %s\t %s", tag.c_str(), data.c_str()); CHIP_ERROR err = ReadString(buffer); - if (err != CHIP_NO_ERROR) { + if (err != CHIP_NO_ERROR) + { ChipLogError(Automation, "Failed to add field: %" CHIP_ERROR_FORMAT, err.Format()); } } From a9c68d0c6f6d1c27dfd19af344c29f11a10b8071 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jul 2023 11:49:05 -0400 Subject: [PATCH 03/13] Ensure sending message data has destination peer address --- examples/common/tracing/TraceDecoder.cpp | 4 ++-- examples/common/tracing/TraceHandlers.cpp | 4 +++- src/transport/SessionManager.cpp | 16 +++++++++++++--- src/transport/TraceMessage.h | 7 ++++--- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/examples/common/tracing/TraceDecoder.cpp b/examples/common/tracing/TraceDecoder.cpp index 9dc35168bdc576..3e5aeaeef41dea 100644 --- a/examples/common/tracing/TraceDecoder.cpp +++ b/examples/common/tracing/TraceDecoder.cpp @@ -185,9 +185,9 @@ CHIP_ERROR TraceDecoder::LogAndConsumeProtocol(Json::Value & json) builder.Add(IsInbound(json) ? "<< from " : ">> to "); - if (json.isMember("peer")) + if (json.isMember("peer_address")) { - builder.Add(json["peer"].asCString()); + builder.Add(json["peer_address"].asCString()); } else { diff --git a/examples/common/tracing/TraceHandlers.cpp b/examples/common/tracing/TraceHandlers.cpp index 0a3e5591e5935d..73f3bb7a71e2e3 100644 --- a/examples/common/tracing/TraceHandlers.cpp +++ b/examples/common/tracing/TraceHandlers.cpp @@ -234,6 +234,8 @@ void SecureMessageSentHandler(const TraceSecureMessageSentData * eventData) } std::string jsonBody = "{ \"direction\": \"outbound\", "; + jsonBody += AsFirstJsonKey("peer_address", AsJsonString(eventData->peerAddress)); + jsonBody += ", "; jsonBody += PacketHeaderToJson(eventData->packetHeader); jsonBody += ", "; jsonBody += PayloadHeaderToJson(eventData->payloadHeader); @@ -256,7 +258,7 @@ void SecureMessageReceivedHandler(const TraceSecureMessageReceivedData * eventDa } std::string jsonBody = "{ \"direction\": \"inbound\", "; - jsonBody += AsFirstJsonKey("peer", AsJsonString(eventData->peerAddress)); + jsonBody += AsFirstJsonKey("peer_address", AsJsonString(eventData->peerAddress)); jsonBody += ", "; jsonBody += PacketHeaderToJson(eventData->packetHeader); jsonBody += ", "; diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 456b0e547aa717..829d0556d2f934 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -147,6 +147,9 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P { MATTER_TRACE_SCOPE("PrepareMessage", "SessionManager"); + // FIXME: Fill it up + PeerAddress destination_address; + PacketHeader packetHeader; bool isControlMsg = IsControlMessage(payloadHeader); if (isControlMsg) @@ -181,11 +184,13 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P return CHIP_ERROR_INTERNAL; } + destination_address = Transport::PeerAddress::Multicast(fabric->GetFabricId(), groupSession->GetGroupId()); + // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kGroupMessage, &payloadHeader, &packetHeader, chip::ByteSpan(message->Start(), message->TotalLength())); - CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, message->Start(), message->TotalLength()); + CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, destination_address, message->Start(), message->TotalLength()); Crypto::SymmetricKeyContext * keyContext = groups->GetKeyContext(groupSession->GetFabricIndex(), groupSession->GetGroupId()); @@ -219,10 +224,12 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P .SetSessionId(session->GetPeerSessionId()) // .SetSessionType(Header::SessionType::kUnicastSession); + destination_address = session->GetPeerAddress(); + // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kSecureSession, &payloadHeader, &packetHeader, chip::ByteSpan(message->Start(), message->TotalLength())); - CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, message->Start(), message->TotalLength()); + CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, destination_address, message->Start(), message->TotalLength()); CryptoContext::NonceStorage nonce; NodeId sourceNodeId = session->GetLocalScopedNodeId().GetNodeId(); @@ -252,10 +259,13 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P break; } + auto unauthenticated = sessionHandle->AsUnauthenticatedSession(); + destination_address = unauthenticated->GetPeerAddress(); + // Trace after all headers are settled. MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kUnauthenticated, &payloadHeader, &packetHeader, chip::ByteSpan(message->Start(), message->TotalLength())); - CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, message->Start(), message->TotalLength()); + CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, destination_address, message->Start(), message->TotalLength()); ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(message)); diff --git a/src/transport/TraceMessage.h b/src/transport/TraceMessage.h index b8c5f87f7c8c1a..61b370f81912ff 100644 --- a/src/transport/TraceMessage.h +++ b/src/transport/TraceMessage.h @@ -40,10 +40,10 @@ #define _CHIP_TRACE_MESSAGE_INTERNAL(type, data, size) PW_TRACE_INSTANT_DATA(::chip::trace::kTraceMessageEvent, type, data, size); #endif // CHIP_CONFIG_TRANSPORT_TRACE_ENABLED && CHIP_CONFIG_TRANSPORT_PW_TRACE_ENABLED -#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, data, dataLen) \ +#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, peerAddress, data, dataLen) \ do \ { \ - const ::chip::trace::TraceSecureMessageSentData _trace_data{ &payloadHeader, &packetHeader, data, dataLen }; \ + const ::chip::trace::TraceSecureMessageSentData _trace_data{ &payloadHeader, &packetHeader, &peerAddress, data, dataLen }; \ _CHIP_TRACE_MESSAGE_INTERNAL(::chip::trace::kTraceMessageSentDataFormat, reinterpret_cast(&_trace_data), \ sizeof(_trace_data)); \ } while (0) @@ -58,7 +58,7 @@ } while (0) #else // CHIP_CONFIG_TRANSPORT_TRACE_ENABLED || CHIP_CONFIG_TRANSPORT_PW_TRACE_ENABLED -#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, data, dataLen) \ +#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, peerAddress data, dataLen) \ do \ { \ } while (0) @@ -81,6 +81,7 @@ struct TraceSecureMessageSentData { const PayloadHeader * payloadHeader; const PacketHeader * packetHeader; + const Transport::PeerAddress * peerAddress; const uint8_t * packetPayload; size_t packetSize; }; From c80469e44e83a8178c9c2ea553c0923dc4cf405d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jul 2023 11:49:19 -0400 Subject: [PATCH 04/13] Restyle --- src/transport/SessionManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 829d0556d2f934..b815a4b2da759b 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -260,7 +260,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P } auto unauthenticated = sessionHandle->AsUnauthenticatedSession(); - destination_address = unauthenticated->GetPeerAddress(); + destination_address = unauthenticated->GetPeerAddress(); // Trace after all headers are settled. MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kUnauthenticated, &payloadHeader, &packetHeader, From e876b88895cfd67ebb8edcc103a8b7a052b96f77 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jul 2023 11:58:45 -0400 Subject: [PATCH 05/13] Add missing comma --- src/transport/TraceMessage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transport/TraceMessage.h b/src/transport/TraceMessage.h index 61b370f81912ff..9063297e7388a8 100644 --- a/src/transport/TraceMessage.h +++ b/src/transport/TraceMessage.h @@ -58,7 +58,7 @@ } while (0) #else // CHIP_CONFIG_TRANSPORT_TRACE_ENABLED || CHIP_CONFIG_TRANSPORT_PW_TRACE_ENABLED -#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, peerAddress data, dataLen) \ +#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, peerAddress, data, dataLen) \ do \ { \ } while (0) From a7750887730983316188aa81047152176c235e05 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jul 2023 12:07:04 -0400 Subject: [PATCH 06/13] Restyle --- src/transport/TraceMessage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transport/TraceMessage.h b/src/transport/TraceMessage.h index 9063297e7388a8..243f3b8479a666 100644 --- a/src/transport/TraceMessage.h +++ b/src/transport/TraceMessage.h @@ -58,7 +58,7 @@ } while (0) #else // CHIP_CONFIG_TRANSPORT_TRACE_ENABLED || CHIP_CONFIG_TRANSPORT_PW_TRACE_ENABLED -#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, peerAddress, data, dataLen) \ +#define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, peerAddress, data, dataLen) \ do \ { \ } while (0) From a63b843c97b1c86f64671aa544de90cf6a194977 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jul 2023 12:21:08 -0400 Subject: [PATCH 07/13] Remove unused constants --- examples/common/tracing/TraceDecoder.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/examples/common/tracing/TraceDecoder.cpp b/examples/common/tracing/TraceDecoder.cpp index 3e5aeaeef41dea..ccc7028bbcff79 100644 --- a/examples/common/tracing/TraceDecoder.cpp +++ b/examples/common/tracing/TraceDecoder.cpp @@ -47,9 +47,6 @@ constexpr const char * kNeedsAckKey = "is_ack_requested"; constexpr const char * kAckMsgKey = "acknowledged_msg_counter"; constexpr const char * kPayloadDataKey = "payload_hex"; constexpr const char * kPayloadSizeKey = "payload_size"; -constexpr const char * kPayloadEncryptedDataKey = "payload_hex_encrypted"; -constexpr const char * kPayloadEncryptedSizeKey = "payload_size_encrypted"; -constexpr const char * kPayloadEncryptedBufferPtrKey = "buffer_ptr"; namespace chip { namespace trace { From efb2fa438b9d7b2e543db57d863cccb3de23a4bb Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 10 Jul 2023 16:21:49 +0000 Subject: [PATCH 08/13] Restyled by clang-format --- examples/common/tracing/TraceDecoder.cpp | 32 ++++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/examples/common/tracing/TraceDecoder.cpp b/examples/common/tracing/TraceDecoder.cpp index ccc7028bbcff79..db3b2cfb076338 100644 --- a/examples/common/tracing/TraceDecoder.cpp +++ b/examples/common/tracing/TraceDecoder.cpp @@ -31,22 +31,22 @@ constexpr uint16_t kMaxLineLen = 4096; constexpr const char * jsonPrefix = " json\t"; // Json keys -constexpr const char * kProtocolIdKey = "protocol_id"; -constexpr const char * kProtocolCodeKey = "protocol_opcode"; -constexpr const char * kSessionIdKey = "session_id"; -constexpr const char * kExchangeIdKey = "exchange_id"; -constexpr const char * kMessageCounterKey = "msg_counter"; -constexpr const char * kSecurityFlagsKey = "security_flags"; -constexpr const char * kMessageFlagsKey = "msg_flags"; -constexpr const char * kSourceNodeIdKey = "source_node_id"; -constexpr const char * kDestinationNodeIdKey = "dest_node_id"; -constexpr const char * kDestinationGroupIdKey = "group_id"; -constexpr const char * kExchangeFlagsKey = "exchange_flags"; -constexpr const char * kIsInitiatorKey = "is_initiator"; -constexpr const char * kNeedsAckKey = "is_ack_requested"; -constexpr const char * kAckMsgKey = "acknowledged_msg_counter"; -constexpr const char * kPayloadDataKey = "payload_hex"; -constexpr const char * kPayloadSizeKey = "payload_size"; +constexpr const char * kProtocolIdKey = "protocol_id"; +constexpr const char * kProtocolCodeKey = "protocol_opcode"; +constexpr const char * kSessionIdKey = "session_id"; +constexpr const char * kExchangeIdKey = "exchange_id"; +constexpr const char * kMessageCounterKey = "msg_counter"; +constexpr const char * kSecurityFlagsKey = "security_flags"; +constexpr const char * kMessageFlagsKey = "msg_flags"; +constexpr const char * kSourceNodeIdKey = "source_node_id"; +constexpr const char * kDestinationNodeIdKey = "dest_node_id"; +constexpr const char * kDestinationGroupIdKey = "group_id"; +constexpr const char * kExchangeFlagsKey = "exchange_flags"; +constexpr const char * kIsInitiatorKey = "is_initiator"; +constexpr const char * kNeedsAckKey = "is_ack_requested"; +constexpr const char * kAckMsgKey = "acknowledged_msg_counter"; +constexpr const char * kPayloadDataKey = "payload_hex"; +constexpr const char * kPayloadSizeKey = "payload_size"; namespace chip { namespace trace { From 7505c08d71a3da1691ccffbf584756c42e96b296 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 11 Jul 2023 09:41:38 -0400 Subject: [PATCH 09/13] Apply code review comments --- examples/common/tracing/TraceDecoder.cpp | 26 ++++++++---------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/examples/common/tracing/TraceDecoder.cpp b/examples/common/tracing/TraceDecoder.cpp index db3b2cfb076338..c2b23fd1e31062 100644 --- a/examples/common/tracing/TraceDecoder.cpp +++ b/examples/common/tracing/TraceDecoder.cpp @@ -30,6 +30,10 @@ constexpr uint16_t kMaxLineLen = 4096; constexpr const char * jsonPrefix = " json\t"; +namespace chip { +namespace trace { +namespace { + // Json keys constexpr const char * kProtocolIdKey = "protocol_id"; constexpr const char * kProtocolCodeKey = "protocol_opcode"; @@ -47,30 +51,18 @@ constexpr const char * kNeedsAckKey = "is_ack_requested"; constexpr const char * kAckMsgKey = "acknowledged_msg_counter"; constexpr const char * kPayloadDataKey = "payload_hex"; constexpr const char * kPayloadSizeKey = "payload_size"; - -namespace chip { -namespace trace { -namespace { - constexpr const char * kDirectionKey = "direction"; +constexpr const char * kPeerAddress = "peer_address"; bool IsOutbound(const Json::Value & json) { - if (!json.isMember(kDirectionKey)) - { - return false; // unknown - } - + VerifyOrReturnValue(json.isMember(kDirectionKey), false); return strcmp(json[kDirectionKey].asCString(), "outbound") == 0; } bool IsInbound(const Json::Value & json) { - if (!json.isMember(kDirectionKey)) - { - return false; // unknown - } - + VerifyOrReturnValue(json.isMember(kDirectionKey), false); return strcmp(json[kDirectionKey].asCString(), "inbound") == 0; } @@ -182,9 +174,9 @@ CHIP_ERROR TraceDecoder::LogAndConsumeProtocol(Json::Value & json) builder.Add(IsInbound(json) ? "<< from " : ">> to "); - if (json.isMember("peer_address")) + if (json.isMember(kPeerAddress)) { - builder.Add(json["peer_address"].asCString()); + builder.Add(json[kPeerAddress].asCString()); } else { From ff6794098222a2981bbd8723d18891ee56956afc Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 11 Jul 2023 13:42:04 +0000 Subject: [PATCH 10/13] Restyled by clang-format --- examples/common/tracing/TraceDecoder.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/common/tracing/TraceDecoder.cpp b/examples/common/tracing/TraceDecoder.cpp index c2b23fd1e31062..7aecfa2bacb08e 100644 --- a/examples/common/tracing/TraceDecoder.cpp +++ b/examples/common/tracing/TraceDecoder.cpp @@ -51,8 +51,8 @@ constexpr const char * kNeedsAckKey = "is_ack_requested"; constexpr const char * kAckMsgKey = "acknowledged_msg_counter"; constexpr const char * kPayloadDataKey = "payload_hex"; constexpr const char * kPayloadSizeKey = "payload_size"; -constexpr const char * kDirectionKey = "direction"; -constexpr const char * kPeerAddress = "peer_address"; +constexpr const char * kDirectionKey = "direction"; +constexpr const char * kPeerAddress = "peer_address"; bool IsOutbound(const Json::Value & json) { From 8248d408036116d43e2f1939e2c4076f87937cc6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 11 Jul 2023 14:57:35 -0400 Subject: [PATCH 11/13] Code review fix --- src/transport/SessionManager.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index b815a4b2da759b..7770dd21dcb1eb 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -147,9 +147,6 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P { MATTER_TRACE_SCOPE("PrepareMessage", "SessionManager"); - // FIXME: Fill it up - PeerAddress destination_address; - PacketHeader packetHeader; bool isControlMsg = IsControlMessage(payloadHeader); if (isControlMsg) @@ -184,7 +181,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P return CHIP_ERROR_INTERNAL; } - destination_address = Transport::PeerAddress::Multicast(fabric->GetFabricId(), groupSession->GetGroupId()); + PeerAddress destination_address = Transport::PeerAddress::Multicast(fabric->GetFabricId(), groupSession->GetGroupId()); // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kGroupMessage, &payloadHeader, &packetHeader, @@ -224,7 +221,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P .SetSessionId(session->GetPeerSessionId()) // .SetSessionType(Header::SessionType::kUnicastSession); - destination_address = session->GetPeerAddress(); + PeerAddress destination_address = session->GetPeerAddress(); // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kSecureSession, &payloadHeader, &packetHeader, @@ -260,7 +257,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P } auto unauthenticated = sessionHandle->AsUnauthenticatedSession(); - destination_address = unauthenticated->GetPeerAddress(); + PeerAddress destination_address = unauthenticated->GetPeerAddress(); // Trace after all headers are settled. MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kUnauthenticated, &payloadHeader, &packetHeader, From bc0f95ece73f070329bb411edb5e2021e23b97b4 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 11 Jul 2023 15:09:41 -0400 Subject: [PATCH 12/13] Avoid unused variable errors --- src/transport/SessionManager.cpp | 9 +++++---- src/transport/TraceMessage.h | 2 ++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 7770dd21dcb1eb..c9f75f405a8375 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -181,7 +181,8 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P return CHIP_ERROR_INTERNAL; } - PeerAddress destination_address = Transport::PeerAddress::Multicast(fabric->GetFabricId(), groupSession->GetGroupId()); + PeerAddress destination_address = + Transport::PeerAddress::Multicast(fabric->GetFabricId(), groupSession->GetGroupId()); // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kGroupMessage, &payloadHeader, &packetHeader, @@ -221,7 +222,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P .SetSessionId(session->GetPeerSessionId()) // .SetSessionType(Header::SessionType::kUnicastSession); - PeerAddress destination_address = session->GetPeerAddress(); + PeerAddress destination_address = session->GetPeerAddress(); // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kSecureSession, &payloadHeader, &packetHeader, @@ -256,8 +257,8 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P break; } - auto unauthenticated = sessionHandle->AsUnauthenticatedSession(); - PeerAddress destination_address = unauthenticated->GetPeerAddress(); + auto unauthenticated = sessionHandle->AsUnauthenticatedSession(); + PeerAddress destination_address = unauthenticated->GetPeerAddress(); // Trace after all headers are settled. MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kUnauthenticated, &payloadHeader, &packetHeader, diff --git a/src/transport/TraceMessage.h b/src/transport/TraceMessage.h index 243f3b8479a666..bf6cbaef99c861 100644 --- a/src/transport/TraceMessage.h +++ b/src/transport/TraceMessage.h @@ -61,11 +61,13 @@ #define CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, peerAddress, data, dataLen) \ do \ { \ + (void) peerAddress; \ } while (0) #define CHIP_TRACE_MESSAGE_RECEIVED(payloadHeader, packetHeader, session, peerAddress, data, dataLen) \ do \ { \ + (void) peerAddress; \ } while (0) #endif // CHIP_CONFIG_TRANSPORT_TRACE_ENABLED || CHIP_CONFIG_TRANSPORT_PW_TRACE_ENABLED From 6bdc29d10e0a318df0dd6c91033897431506df0a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 11 Jul 2023 15:09:56 -0400 Subject: [PATCH 13/13] Restyle --- src/transport/SessionManager.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index c9f75f405a8375..7b9ebaf0ad8f68 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -181,8 +181,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P return CHIP_ERROR_INTERNAL; } - PeerAddress destination_address = - Transport::PeerAddress::Multicast(fabric->GetFabricId(), groupSession->GetGroupId()); + PeerAddress destination_address = Transport::PeerAddress::Multicast(fabric->GetFabricId(), groupSession->GetGroupId()); // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kGroupMessage, &payloadHeader, &packetHeader, @@ -222,7 +221,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P .SetSessionId(session->GetPeerSessionId()) // .SetSessionType(Header::SessionType::kUnicastSession); - PeerAddress destination_address = session->GetPeerAddress(); + PeerAddress destination_address = session->GetPeerAddress(); // Trace before any encryption MATTER_LOG_MESSAGE_SEND(chip::Tracing::OutgoingMessageType::kSecureSession, &payloadHeader, &packetHeader, @@ -257,7 +256,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P break; } - auto unauthenticated = sessionHandle->AsUnauthenticatedSession(); + auto unauthenticated = sessionHandle->AsUnauthenticatedSession(); PeerAddress destination_address = unauthenticated->GetPeerAddress(); // Trace after all headers are settled.