From 5070adbc298966f001208c417411fd00a67d332e Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 17 Sep 2024 14:06:06 -0700 Subject: [PATCH] [settings] simplify saving of Border Agent ID (#10701) This commit simplifies how Border Agent ID is saved in non-volatile `Settings` by utilizing generic methods designed for single-value setting entries. As a result, `Settings::BorderAgentId` now only needs to define the key and the associated entry value type. This eliminates the need for `otBorderAgentId` to be defined as packed, thereby simplifying this structure. --- include/openthread/border_agent.h | 17 ++++---------- include/openthread/instance.h | 2 +- src/core/common/settings.cpp | 15 +++++++------ src/core/common/settings.hpp | 37 +++++-------------------------- src/core/meshcop/border_agent.cpp | 35 +++++++++++++---------------- 5 files changed, 34 insertions(+), 72 deletions(-) diff --git a/include/openthread/border_agent.h b/include/openthread/border_agent.h index 38bc1dc9c..e41d026bf 100644 --- a/include/openthread/border_agent.h +++ b/include/openthread/border_agent.h @@ -82,22 +82,13 @@ extern "C" { #define OT_BORDER_AGENT_MAX_EPHEMERAL_KEY_TIMEOUT (10 * 60 * 1000u) /** - * @struct otBorderAgentId - * - * Represents a Border Agent ID. + * Represents a Border Agent Identifier. * */ -OT_TOOL_PACKED_BEGIN -struct otBorderAgentId +typedef struct otBorderAgentId { - uint8_t mId[OT_BORDER_AGENT_ID_LENGTH]; -} OT_TOOL_PACKED_END; - -/** - * Represents a Border Agent ID. - * - */ -typedef struct otBorderAgentId otBorderAgentId; + uint8_t mId[OT_BORDER_AGENT_ID_LENGTH]; ///< Border Agent ID bytes. +} otBorderAgentId; /** * Defines the Border Agent state. diff --git a/include/openthread/instance.h b/include/openthread/instance.h index cf7e893dd..3d005568e 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (445) +#define OPENTHREAD_API_VERSION (446) /** * @addtogroup api-instance diff --git a/src/core/common/settings.cpp b/src/core/common/settings.cpp index 695e899bf..53096c2a3 100644 --- a/src/core/common/settings.cpp +++ b/src/core/common/settings.cpp @@ -107,15 +107,16 @@ void SettingsBase::SrpServerInfo::Log(Action aAction) const #endif #if OPENTHREAD_CONFIG_BORDER_AGENT_ENABLE && OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE -void SettingsBase::BorderAgentId::Log(Action aAction) const +void SettingsBase::BorderAgentId::Log(Action aAction, const MeshCoP::BorderAgent::Id &aId) { - char buffer[sizeof(BorderAgentId) * 2 + 1]; - StringWriter sw(buffer, sizeof(buffer)); + static constexpr uint8_t kStringSize = sizeof(MeshCoP::BorderAgent::Id) * 2 + 1; - sw.AppendHexBytes(GetId().mId, sizeof(BorderAgentId)); - LogInfo("%s BorderAgentId {id:%s}", ActionToString(aAction), buffer); + String string; + + string.AppendHexBytes(aId.mId, sizeof(aId)); + LogInfo("%s BorderAgentId {id:%s}", ActionToString(aAction), string.AsCString()); } -#endif // OPENTHREAD_CONFIG_BORDER_AGENT_ENABLE && OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE +#endif #endif // OT_SHOULD_LOG_AT(OT_LOG_LEVEL_INFO) @@ -533,7 +534,7 @@ void Settings::Log(Action aAction, Error aError, Key aKey, const void *aValue) #if OPENTHREAD_CONFIG_BORDER_AGENT_ENABLE && OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE case kKeyBorderAgentId: - reinterpret_cast(aValue)->Log(aAction); + BorderAgentId::Log(aAction, *reinterpret_cast(aValue)); break; #endif diff --git a/src/core/common/settings.hpp b/src/core/common/settings.hpp index c98829c34..8b2fede0f 100644 --- a/src/core/common/settings.hpp +++ b/src/core/common/settings.hpp @@ -772,7 +772,6 @@ public: * Represents the Border Agent ID. * */ - OT_TOOL_PACKED_BEGIN class BorderAgentId { friend class Settings; @@ -780,40 +779,14 @@ public: public: static constexpr Key kKey = kKeyBorderAgentId; ///< The associated key. - /** - * Initializes the `BorderAgentId` object. - * - */ - void Init(void) { ClearAllBytes(mId); } - - /** - * Returns the Border Agent ID. - * - * @returns The Border Agent ID. - * - */ - const MeshCoP::BorderAgent::Id &GetId(void) const { return mId; } - - /** - * Returns the Border Agent ID. - * - * @returns The Border Agent ID. - * - */ - MeshCoP::BorderAgent::Id &GetId(void) { return mId; } - - /** - * Sets the Border Agent ID. - * - */ - void SetId(const MeshCoP::BorderAgent::Id &aId) { mId = aId; } + typedef MeshCoP::BorderAgent::Id ValueType; ///< The associated value type. private: - void Log(Action aAction) const; + static void Log(Action aAction, const MeshCoP::BorderAgent::Id &aId); - MeshCoP::BorderAgent::Id mId; - } OT_TOOL_PACKED_END; -#endif // OPENTHREAD_CONFIG_BORDER_AGENT_ENABLE && OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE + BorderAgentId(void) = delete; + }; +#endif protected: explicit SettingsBase(Instance &aInstance) diff --git a/src/core/meshcop/border_agent.cpp b/src/core/meshcop/border_agent.cpp index 20cf45d7b..611607e25 100644 --- a/src/core/meshcop/border_agent.cpp +++ b/src/core/meshcop/border_agent.cpp @@ -268,35 +268,32 @@ BorderAgent::BorderAgent(Instance &aInstance) #if OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE Error BorderAgent::GetId(Id &aId) { - Error error = kErrorNone; - Settings::BorderAgentId id; + Error error = kErrorNone; - VerifyOrExit(!mIdInitialized, error = kErrorNone); - - if (Get().Read(id) != kErrorNone) - { - Random::NonCrypto::Fill(id.GetId()); - SuccessOrExit(error = Get().Save(id)); - } - - mId = id.GetId(); - mIdInitialized = true; - -exit: - if (error == kErrorNone) + if (mIdInitialized) { aId = mId; + ExitNow(); } + + if (Get().Read(mId) != kErrorNone) + { + Random::NonCrypto::Fill(mId); + SuccessOrExit(error = Get().Save(mId)); + } + + mIdInitialized = true; + aId = mId; + +exit: return error; } Error BorderAgent::SetId(const Id &aId) { - Error error = kErrorNone; - Settings::BorderAgentId id; + Error error = kErrorNone; - id.SetId(aId); - SuccessOrExit(error = Get().Save(id)); + SuccessOrExit(error = Get().Save(aId)); mId = aId; mIdInitialized = true;