From 5657335de4f02db054edf80882a015afda58de15 Mon Sep 17 00:00:00 2001 From: Nick Bertoldi <109968900+bertoldi-silabs@users.noreply.github.com> Date: Wed, 26 Apr 2023 12:07:30 -0400 Subject: [PATCH] [cli] store cli user commands in container and support list prepend (#8977) Calls to otCliSetUserCommand overwrite the pointer user command list pointer each time it is invoked, which results in successive calls to otCliSetUserCommand replacing the previously registered command list instead of appending to it. This is particularly relevant for the posix platform in which the main function registers a set of posix specific commands without any way to extend the set. This commit replaces the command list pointer with a container of command lists associated with a registered context in order to support prepending the active list of user commands up to a configurable value which defaults to 1 to maintain current behavior. --- examples/apps/cli/main.c | 2 +- include/openthread/cli.h | 4 ++- include/openthread/instance.h | 2 +- src/cli/cli.cpp | 50 ++++++++++++++++++++++++----------- src/cli/cli.hpp | 29 +++++++++++++------- src/cli/cli_config.h | 10 +++++++ src/posix/main.c | 2 +- 7 files changed, 69 insertions(+), 30 deletions(-) diff --git a/examples/apps/cli/main.c b/examples/apps/cli/main.c index c680628ca..cc52d9ab7 100644 --- a/examples/apps/cli/main.c +++ b/examples/apps/cli/main.c @@ -126,7 +126,7 @@ pseudo_reset: otAppCliInit(instance); #if OPENTHREAD_POSIX && !defined(FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION) - otCliSetUserCommands(kCommands, OT_ARRAY_LENGTH(kCommands), instance); + IgnoreError(otCliSetUserCommands(kCommands, OT_ARRAY_LENGTH(kCommands), instance)); #endif while (!otSysPseudoResetWasRequested()) diff --git a/include/openthread/cli.h b/include/openthread/cli.h index 4a80c9e14..e0d9e66ee 100644 --- a/include/openthread/cli.h +++ b/include/openthread/cli.h @@ -104,8 +104,10 @@ void otCliInputLine(char *aBuf); * @param[in] aLength @p aUserCommands length. * @param[in] aContext @p The context passed to the handler. * + * @retval OT_ERROR_NONE Successfully updated command table with commands from @p aUserCommands. + * @retval OT_ERROR_FAILED Maximum number of command entries have already been set. */ -void otCliSetUserCommands(const otCliCommand *aUserCommands, uint8_t aLength, void *aContext); +otError otCliSetUserCommands(const otCliCommand *aUserCommands, uint8_t aLength, void *aContext); /** * Write a number of bytes to the CLI console as a hex string. diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 16e26126c..257ae4a0e 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 (311) +#define OPENTHREAD_API_VERSION (312) /** * @addtogroup api-instance diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index 3c66637d2..4ab4b36a4 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -100,8 +100,6 @@ static OT_DEFINE_ALIGNED_VAR(sInterpreterRaw, sizeof(Interpreter), uint64_t); Interpreter::Interpreter(Instance *aInstance, otCliOutputCallback aCallback, void *aContext) : OutputImplementer(aCallback, aContext) , Output(aInstance, *this) - , mUserCommands(nullptr) - , mUserCommandsLength(0) , mCommandIsPending(false) , mTimer(*aInstance, HandleTimer, this) #if OPENTHREAD_FTD || OPENTHREAD_MTD @@ -149,6 +147,7 @@ Interpreter::Interpreter(Instance *aInstance, otCliOutputCallback aCallback, voi #if (OPENTHREAD_FTD || OPENTHREAD_MTD) && OPENTHREAD_CONFIG_CLI_REGISTER_IP6_RECV_CALLBACK otIp6SetReceiveCallback(GetInstancePtr(), &Interpreter::HandleIp6Receive, this); #endif + memset(&mUserCommands, 0, sizeof(mUserCommands)); OutputPrompt(); } @@ -295,26 +294,42 @@ otError Interpreter::ProcessUserCommands(Arg aArgs[]) { otError error = OT_ERROR_INVALID_COMMAND; - for (uint8_t i = 0; i < mUserCommandsLength; i++) + for (const UserCommandsEntry &entry : mUserCommands) { - if (aArgs[0] == mUserCommands[i].mName) + for (uint8_t i = 0; i < entry.mLength; i++) { - char *args[kMaxArgs]; + if (aArgs[0] == entry.mCommands[i].mName) + { + char *args[kMaxArgs]; - Arg::CopyArgsToStringArray(aArgs, args); - error = mUserCommands[i].mCommand(mUserCommandsContext, Arg::GetArgsLength(aArgs) - 1, args + 1); - break; + Arg::CopyArgsToStringArray(aArgs, args); + error = entry.mCommands[i].mCommand(entry.mContext, Arg::GetArgsLength(aArgs) - 1, args + 1); + break; + } } } return error; } -void Interpreter::SetUserCommands(const otCliCommand *aCommands, uint8_t aLength, void *aContext) +otError Interpreter::SetUserCommands(const otCliCommand *aCommands, uint8_t aLength, void *aContext) { - mUserCommands = aCommands; - mUserCommandsLength = aLength; - mUserCommandsContext = aContext; + otError error = OT_ERROR_FAILED; + + for (UserCommandsEntry &entry : mUserCommands) + { + if (entry.mCommands == nullptr) + { + entry.mCommands = aCommands; + entry.mLength = aLength; + entry.mContext = aContext; + + error = OT_ERROR_NONE; + break; + } + } + + return error; } #if OPENTHREAD_FTD || OPENTHREAD_MTD @@ -8507,9 +8522,12 @@ otError Interpreter::ProcessCommand(Arg aArgs[]) { OutputCommandTable(kCommands); - for (uint8_t i = 0; i < mUserCommandsLength; i++) + for (const UserCommandsEntry &entry : mUserCommands) { - OutputLine("%s", mUserCommands[i].mName); + for (uint8_t i = 0; i < entry.mLength; i++) + { + OutputLine("%s", entry.mCommands[i].mName); + } } } else @@ -8527,9 +8545,9 @@ extern "C" void otCliInit(otInstance *aInstance, otCliOutputCallback aCallback, extern "C" void otCliInputLine(char *aBuf) { Interpreter::GetInterpreter().ProcessLine(aBuf); } -extern "C" void otCliSetUserCommands(const otCliCommand *aUserCommands, uint8_t aLength, void *aContext) +extern "C" otError otCliSetUserCommands(const otCliCommand *aUserCommands, uint8_t aLength, void *aContext) { - Interpreter::GetInterpreter().SetUserCommands(aUserCommands, aLength, aContext); + return Interpreter::GetInterpreter().SetUserCommands(aUserCommands, aLength, aContext); } extern "C" void otCliOutputBytes(const uint8_t *aBytes, uint8_t aLength) diff --git a/src/cli/cli.hpp b/src/cli/cli.hpp index faa3d41c4..6f880ad3f 100644 --- a/src/cli/cli.hpp +++ b/src/cli/cli.hpp @@ -76,6 +76,7 @@ #include "cli/cli_coap_secure.hpp" #endif +#include "common/array.hpp" #include "common/code_utils.hpp" #include "common/debug.hpp" #include "common/instance.hpp" @@ -180,14 +181,16 @@ public: static otError ParseEnableOrDisable(const Arg &aArg, bool &aEnable); /** - * This method sets the user command table. + * This method adds commands to the user command table. * * @param[in] aCommands A pointer to an array with user commands. * @param[in] aLength @p aUserCommands length. * @param[in] aContext @p aUserCommands length. * + * @retval OT_ERROR_NONE Successfully updated command table with commands from @p aCommands. + * @retval OT_ERROR_FAILED No available UserCommandsEntry to register requested user commands. */ - void SetUserCommands(const otCliCommand *aCommands, uint8_t aLength, void *aContext); + otError SetUserCommands(const otCliCommand *aCommands, uint8_t aLength, void *aContext); static constexpr uint8_t kLinkModeStringSize = sizeof("rdn"); ///< Size of string buffer for a MLE Link Mode. @@ -265,10 +268,11 @@ protected: private: enum { - kIndentSize = 4, - kMaxArgs = 32, - kMaxAutoAddresses = 8, - kMaxLineLength = OPENTHREAD_CONFIG_CLI_MAX_LINE_LENGTH, + kIndentSize = 4, + kMaxArgs = 32, + kMaxAutoAddresses = 8, + kMaxLineLength = OPENTHREAD_CONFIG_CLI_MAX_LINE_LENGTH, + kMaxUserCommandEntries = OPENTHREAD_CONFIG_CLI_MAX_USER_CMD_ENTRIES, }; static constexpr uint32_t kNetworkDiagnosticTimeoutMsecs = 5000; @@ -531,10 +535,15 @@ private: static void HandleTimer(Timer &aTimer); void HandleTimer(void); - const otCliCommand *mUserCommands; - uint8_t mUserCommandsLength; - void *mUserCommandsContext; - bool mCommandIsPending; + struct UserCommandsEntry + { + const otCliCommand *mCommands; + uint8_t mLength; + void *mContext; + }; + + UserCommandsEntry mUserCommands[kMaxUserCommandEntries]; + bool mCommandIsPending; TimerMilliContext mTimer; diff --git a/src/cli/cli_config.h b/src/cli/cli_config.h index 15b444bb2..4e0305259 100644 --- a/src/cli/cli_config.h +++ b/src/cli/cli_config.h @@ -87,6 +87,16 @@ #define OPENTHREAD_CONFIG_CLI_TCP_RECEIVE_BUFFER_SIZE OT_TCP_RECEIVE_BUFFER_SIZE_FEW_HOPS #endif +/** + * @def OPENTHREAD_CONFIG_CLI_MAX_USER_CMD_ENTRIES + * + * The maximum number of user CLI command lists that can be registered by the interpreter. + * + */ +#ifndef OPENTHREAD_CONFIG_CLI_MAX_USER_CMD_ENTRIES +#define OPENTHREAD_CONFIG_CLI_MAX_USER_CMD_ENTRIES 1 +#endif + /** * @def OPENTHREAD_CONFIG_CLI_LOG_INPUT_OUTPUT_ENABLE * diff --git a/src/posix/main.c b/src/posix/main.c index 9b81e2d0f..8800ac0c1 100644 --- a/src/posix/main.c +++ b/src/posix/main.c @@ -380,7 +380,7 @@ int main(int argc, char *argv[]) #if !OPENTHREAD_POSIX_CONFIG_DAEMON_ENABLE otAppCliInit(instance); #endif - otCliSetUserCommands(kCommands, OT_ARRAY_LENGTH(kCommands), instance); + IgnoreError(otCliSetUserCommands(kCommands, OT_ARRAY_LENGTH(kCommands), instance)); while (true) {