From 15f4e0129edaa2fa4219057e263c5ed57c227d59 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 1 Mar 2018 13:49:13 +0000 Subject: [PATCH] [meshcop] improve logging of adding/removing joiners (#2597) This commit makes the following changes: - Only print logs on success. - Remove use of `HostSwap64()`, which can cause hard faults. - Add `ExtAddress::ToString()` method. --- src/core/mac/mac_frame.cpp | 14 +++++++--- src/core/mac/mac_frame.hpp | 11 ++++++++ src/core/meshcop/commissioner.cpp | 45 ++++++++++++++++++++++++------- src/core/meshcop/commissioner.hpp | 6 +---- src/core/meshcop/joiner.cpp | 7 ++--- 5 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp index 43b98820a..0a9d41777 100644 --- a/src/core/mac/mac_frame.cpp +++ b/src/core/mac/mac_frame.cpp @@ -53,6 +53,12 @@ bool ExtAddress::operator!=(const ExtAddress &aOther) const return memcmp(m8, aOther.m8, sizeof(ExtAddress)) != 0; } +const char *ExtAddress::ToString(char *aBuf, uint16_t aSize) const +{ + snprintf(aBuf, aSize, "%02x%02x%02x%02x%02x%02x%02x%02x", m8[0], m8[1], m8[2], m8[3], m8[4], m8[5], m8[6], m8[7]); + return aBuf; +} + void Address::SetExtended(const uint8_t *aBuffer, bool aReverse) { mType = kTypeExtended; @@ -72,6 +78,8 @@ void Address::SetExtended(const uint8_t *aBuffer, bool aReverse) const char *Address::ToString(char *aBuf, uint16_t aSize) const { + const char *rval = aBuf; + switch (mType) { case kTypeNone: @@ -83,13 +91,11 @@ const char *Address::ToString(char *aBuf, uint16_t aSize) const break; case kTypeExtended: - snprintf(aBuf, aSize, "%02x%02x%02x%02x%02x%02x%02x%02x", GetExtended().m8[0], GetExtended().m8[1], - GetExtended().m8[2], GetExtended().m8[3], GetExtended().m8[4], GetExtended().m8[5], - GetExtended().m8[6], GetExtended().m8[7]); + rval = GetExtended().ToString(aBuf, aSize); break; } - return aBuf; + return rval; } otError Frame::InitMacHeader(uint16_t aFcf, uint8_t aSecurityControl) diff --git a/src/core/mac/mac_frame.hpp b/src/core/mac/mac_frame.hpp index 39bef3be1..2990959a3 100644 --- a/src/core/mac/mac_frame.hpp +++ b/src/core/mac/mac_frame.hpp @@ -171,6 +171,17 @@ public: */ bool operator!=(const ExtAddress &aOther) const; + /** + * This method converts an address to a NULL-terminated string. + * + * @param[out] aBuf A pointer to a character buffer. + * @param[in] aSize The maximum size of the buffer. + * + * @returns A pointer to the character string buffer. + * + */ + const char *ToString(char *aBuf, uint16_t aSize) const; + private: enum { diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index f11243d11..7ffe4d1aa 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/src/core/meshcop/commissioner.cpp @@ -56,8 +56,6 @@ #if OPENTHREAD_FTD && OPENTHREAD_ENABLE_COMMISSIONER -using ot::Encoding::BigEndian::HostSwap64; - namespace ot { namespace MeshCoP { @@ -203,9 +201,6 @@ otError Commissioner::AddJoiner(const Mac::ExtAddress *aEui64, const char *aPSKd VerifyOrExit(mState == OT_COMMISSIONER_STATE_ACTIVE, error = OT_ERROR_INVALID_STATE); - otLogInfoMeshCoP(GetInstance(), "AddJoiner() %llX, %s", - (aEui64 ? HostSwap64(*reinterpret_cast(aEui64)) : 0), aPSKd); - VerifyOrExit(strlen(aPSKd) <= Dtls::kPskMaxLength, error = OT_ERROR_INVALID_ARGS); RemoveJoiner(aEui64, 0); // remove immediately @@ -238,6 +233,23 @@ otError Commissioner::AddJoiner(const Mac::ExtAddress *aEui64, const char *aPSKd } exit: + if (error == OT_ERROR_NONE) + { + if (aEui64) + { + char logString[Mac::Address::kAddressStringSize]; + + otLogInfoMeshCoP(GetInstance(), "Added Joiner (%s, %s)", aEui64->ToString(logString, sizeof(logString)), + aPSKd); + + OT_UNUSED_VARIABLE(logString); + } + else + { + otLogInfoMeshCoP(GetInstance(), "Added Joiner (*, %s)", aPSKd); + } + } + return error; } @@ -247,9 +259,6 @@ otError Commissioner::RemoveJoiner(const Mac::ExtAddress *aEui64, uint32_t aDela VerifyOrExit(mState == OT_COMMISSIONER_STATE_ACTIVE, error = OT_ERROR_INVALID_STATE); - otLogInfoMeshCoP(GetInstance(), "RemoveJoiner() %llX", - (aEui64 ? HostSwap64(*reinterpret_cast(aEui64)) : 0)); - for (size_t i = 0; i < sizeof(mJoiners) / sizeof(mJoiners[0]); i++) { if (!mJoiners[i].mValid) @@ -291,6 +300,22 @@ otError Commissioner::RemoveJoiner(const Mac::ExtAddress *aEui64, uint32_t aDela } exit: + if (error == OT_ERROR_NONE) + { + if (aEui64) + { + char logString[Mac::Address::kAddressStringSize]; + + otLogInfoMeshCoP(GetInstance(), "Removed Joiner (%s)", aEui64->ToString(logString, sizeof(logString))); + + OT_UNUSED_VARIABLE(logString); + } + else + { + otLogInfoMeshCoP(GetInstance(), "Removed Joiner (*)"); + } + } + return error; } @@ -828,7 +853,9 @@ void Commissioner::HandleRelayReceive(Coap::Header &aHeader, Message &aMessage, mJoinerPort = joinerPort.GetUdpPort(); mJoinerRloc = joinerRloc.GetJoinerRouterLocator(); - otLogInfoMeshCoP(GetInstance(), "Received relay receive for %llX, rloc:%x", HostSwap64(mJoinerIid64), mJoinerRloc); + otLogInfoMeshCoP(GetInstance(), "Remove Relay Receive (%02x%02x%02x%02x%02x%02x%02x%02x, 0x%04x)", mJoinerIid[0], + mJoinerIid[1], mJoinerIid[2], mJoinerIid[3], mJoinerIid[4], mJoinerIid[5], mJoinerIid[6], + mJoinerIid[7], mJoinerRloc); aMessage.SetOffset(offset); SuccessOrExit(error = aMessage.SetLength(offset + length)); diff --git a/src/core/meshcop/commissioner.hpp b/src/core/meshcop/commissioner.hpp index b53c0ab70..5a2f34615 100644 --- a/src/core/meshcop/commissioner.hpp +++ b/src/core/meshcop/commissioner.hpp @@ -318,11 +318,7 @@ private: }; Joiner mJoiners[OPENTHREAD_CONFIG_MAX_JOINER_ENTRIES]; - union - { - uint8_t mJoinerIid[8]; - uint64_t mJoinerIid64; - }; + uint8_t mJoinerIid[8]; uint16_t mJoinerPort; uint16_t mJoinerRloc; TimerMilli mJoinerExpirationTimer; diff --git a/src/core/meshcop/joiner.cpp b/src/core/meshcop/joiner.cpp index 62ce6aff5..4364e648e 100644 --- a/src/core/meshcop/joiner.cpp +++ b/src/core/meshcop/joiner.cpp @@ -55,7 +55,6 @@ #if OPENTHREAD_ENABLE_JOINER using ot::Encoding::BigEndian::HostSwap16; -using ot::Encoding::BigEndian::HostSwap64; namespace ot { namespace MeshCoP { @@ -197,9 +196,11 @@ void Joiner::HandleDiscoverResult(otActiveScanResult *aResult) if (aResult != NULL) { JoinerRouter joinerRouter; + char logString[Mac::Address::kAddressStringSize]; - otLogDebgMeshCoP(GetInstance(), "HandleDiscoverResult() aResult = %llX", - HostSwap64(*reinterpret_cast(&aResult->mExtAddress))); + otLogDebgMeshCoP(GetInstance(), "Received Discovery Response (%s)", + static_cast(aResult->mExtAddress).ToString(logString, sizeof(logString))); + OT_UNUSED_VARIABLE(logString); // Joining is disabled if the Steering Data is not included if (aResult->mSteeringData.mLength == 0)