From a6a0f1c213e369c6414c7bc6fb5bca5e5ee1f690 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 16 Jan 2018 09:45:39 -0800 Subject: [PATCH] [diags] fix parsing command line and enhancements (#2480) This commit contains fixes and enchantments for diagnostics module in OpenThread. In particular, the following changes are made to the implementation of `otDiagProcessCmdLine()`: - Fixes an issue with the parsing of command line input (where if more than 8 arguments were given, it could cause an out-of bound array access and a possible NCP crash). The new code will check for this and outputs an error if too many arguments are provided. - Parsing logic is simplified and now allows for extra spaces between arguments. - A local buffer is used to store the arguments to avoid modifying the passed-in input string. This commit also simplifies and does a code clean-up of `Diag` class (variable name changes, removing extra method paramters, and minor style changes). The `test_diag` unit test implementation is also updated: - It includes new test commands. - The test prints/logs the issued input commands and their corresponding output (as human-readable printable strings). --- include/openthread/diag.h | 10 +- src/diag/diag_process.cpp | 212 +++++++++++++++++------------------ src/diag/diag_process.hpp | 60 +++++----- src/diag/openthread-diag.cpp | 92 ++++++++++++--- src/ncp/ncp_base_mtd.cpp | 5 +- tests/unit/test_diag.cpp | 117 +++++++++++++++---- 6 files changed, 317 insertions(+), 179 deletions(-) diff --git a/include/openthread/diag.h b/include/openthread/diag.h index 844251aaf..9e67e57f6 100644 --- a/include/openthread/diag.h +++ b/include/openthread/diag.h @@ -62,23 +62,23 @@ void otDiagInit(otInstance *aInstance); /** * This function processes a factory diagnostics command line. * - * @param[in] aArgc The argument counter of diagnostics command line. - * @param[in] aArgv The argument vector of diagnostics command line. + * @param[in] aArgCount The argument counter of diagnostics command line. + * @param[in] aArgVector The argument vector of diagnostics command line. * * @returns A pointer to the output string. * */ -char *otDiagProcessCmd(int aArgc, char *aArgv[]); +const char *otDiagProcessCmd(int aArgCount, char *aArgVector[]); /** * This function processes a factory diagnostics command line. * - * @param[in] aString A NULL-terminated string. + * @param[in] aString A NULL-terminated input string. * * @returns A pointer to the output string. * */ -char *otDiagProcessCmdLine(char *aString); +const char *otDiagProcessCmdLine(const char *aString); /** * This function indicates whether or not the factory diagnostics mode is enabled. diff --git a/src/diag/diag_process.cpp b/src/diag/diag_process.cpp index 533371c64..af16ffcca 100644 --- a/src/diag/diag_process.cpp +++ b/src/diag/diag_process.cpp @@ -44,7 +44,7 @@ namespace ot { namespace Diagnostics { -const struct Command Diag::sCommands[] = +const struct Diag::Command Diag::sCommands[] = { { "start", &ProcessStart }, { "stop", &ProcessStop }, @@ -53,10 +53,11 @@ const struct Command Diag::sCommands[] = { "send", &ProcessSend }, { "repeat", &ProcessRepeat }, { "stats", &ProcessStats }, + { NULL, NULL }, }; -char Diag::sDiagOutput[MAX_DIAG_OUTPUT]; -struct DiagStats Diag::sStats; +char Diag::sOutput[Diag::kMaxOutputSize]; +struct Diag::DiagStats Diag::sStats; int8_t Diag::sTxPower; uint8_t Diag::sChannel; @@ -65,12 +66,11 @@ uint32_t Diag::sTxPeriod; uint32_t Diag::sTxPackets; otRadioFrame * Diag::sTxPacket; bool Diag::sRepeatActive; - -otInstance *Diag::sContext; +otInstance *Diag::sInstance; void Diag::Init(otInstance *aInstance) { - sContext = aInstance; + sInstance = aInstance; sChannel = 20; sTxPower = 0; sTxPeriod = 0; @@ -78,110 +78,98 @@ void Diag::Init(otInstance *aInstance) sTxPackets = 0; sRepeatActive = false; memset(&sStats, 0, sizeof(struct DiagStats)); - sTxPacket = otPlatRadioGetTransmitBuffer(sContext); + sTxPacket = otPlatRadioGetTransmitBuffer(sInstance); otPlatDiagChannelSet(sChannel); otPlatDiagTxPowerSet(sTxPower); } -char *Diag::ProcessCmd(int argc, char *argv[]) +const char *Diag::ProcessCmd(int aArgCount, char *aArgVector[]) { - unsigned int i; - - if (argc == 0) + if (aArgCount == 0) { - snprintf(sDiagOutput, sizeof(sDiagOutput), "diagnostics mode is %s\r\n", otPlatDiagModeGet() ? "enabled" : "disabled"); + snprintf(sOutput, sizeof(sOutput), "diagnostics mode is %s\r\n", otPlatDiagModeGet() ? "enabled" : "disabled"); + ExitNow(); } - else - { - for (i = 0; i < sizeof(sCommands) / sizeof(sCommands[0]); i++) - { - if (strcmp(argv[0], sCommands[i].mName) == 0) - { - sCommands[i].mCommand(argc - 1, argc > 1 ? &argv[1] : NULL, sDiagOutput, sizeof(sDiagOutput)); - break; - } - } - // more platform specific features will be processed under platform layer - if (i == sizeof(sCommands) / sizeof(sCommands[0])) + for (const Command *command = &sCommands[0]; command->mName != NULL; command++) + { + if (strcmp(aArgVector[0], command->mName) == 0) { - otPlatDiagProcess(sContext, argc, argv, sDiagOutput, sizeof(sDiagOutput)); + command->mHandler(aArgCount - 1, (aArgCount > 1) ? &aArgVector[1] : NULL); + ExitNow(); } } - return sDiagOutput; + // more platform specific features will be processed under platform layer + otPlatDiagProcess(sInstance, aArgCount, aArgVector, sOutput, sizeof(sOutput)); + +exit: + return sOutput; } -bool Diag::isEnabled() +bool Diag::IsEnabled(void) { return otPlatDiagModeGet(); } -void Diag::AppendErrorResult(otError aError, char *aOutput, size_t aOutputMaxLen) +void Diag::AppendErrorResult(otError aError) { if (aError != OT_ERROR_NONE) { - snprintf(aOutput, aOutputMaxLen, "failed\r\nstatus %#x\r\n", aError); + snprintf(sOutput, sizeof(sOutput), "failed\r\nstatus %#x\r\n", aError); } } -void Diag::ProcessStart(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen) +void Diag::ProcessStart(int aArgCount, char *aArgVector[]) { otError error = OT_ERROR_NONE; - // enable radio - otPlatRadioEnable(sContext); + OT_UNUSED_VARIABLE(aArgCount); + OT_UNUSED_VARIABLE(aArgVector); - // enable promiscuous mode - otPlatRadioSetPromiscuous(sContext, true); - - // stop timer - otPlatAlarmMilliStop(sContext); - - // start to listen on the default channel - SuccessOrExit(error = otPlatRadioReceive(sContext, sChannel)); - - // enable diagnostics mode + otPlatRadioEnable(sInstance); + otPlatRadioSetPromiscuous(sInstance, true); + otPlatAlarmMilliStop(sInstance); + SuccessOrExit(error = otPlatRadioReceive(sInstance, sChannel)); otPlatDiagModeSet(true); - memset(&sStats, 0, sizeof(struct DiagStats)); - - snprintf(aOutput, aOutputMaxLen, "start diagnostics mode\r\nstatus 0x%02x\r\n", error); + snprintf(sOutput, sizeof(sOutput), "start diagnostics mode\r\nstatus 0x%02x\r\n", error); exit: - OT_UNUSED_VARIABLE(argc); - OT_UNUSED_VARIABLE(argv); - AppendErrorResult(error, aOutput, aOutputMaxLen); + AppendErrorResult(error); } -void Diag::ProcessStop(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen) +void Diag::ProcessStop(int aArgCount, char *aArgVector[]) { otError error = OT_ERROR_NONE; + OT_UNUSED_VARIABLE(aArgCount); + OT_UNUSED_VARIABLE(aArgVector); + VerifyOrExit(otPlatDiagModeGet(), error = OT_ERROR_INVALID_STATE); - otPlatAlarmMilliStop(sContext); + otPlatAlarmMilliStop(sInstance); otPlatDiagModeSet(false); - otPlatRadioSetPromiscuous(sContext, false); + otPlatRadioSetPromiscuous(sInstance, false); - snprintf(aOutput, aOutputMaxLen, "received packets: %d\r\nsent packets: %d\r\nfirst received packet: rssi=%d, lqi=%d\r\n\nstop diagnostics mode\r\nstatus 0x%02x\r\n", - static_cast(sStats.received_packets), static_cast(sStats.sent_packets), static_cast(sStats.first_rssi), - static_cast(sStats.first_lqi), error); + snprintf(sOutput, sizeof(sOutput), + "received packets: %d\r\nsent packets: %d\r\nfirst received packet: rssi=%d, lqi=%d\r\n" + "\nstop diagnostics mode\r\nstatus 0x%02x\r\n", + static_cast(sStats.mReceivedPackets), static_cast(sStats.mSentPackets), + static_cast(sStats.mFirstRssi), static_cast(sStats.mFirstLqi), error); exit: - OT_UNUSED_VARIABLE(argc); - OT_UNUSED_VARIABLE(argv); - AppendErrorResult(error, aOutput, aOutputMaxLen); + AppendErrorResult(error); } -otError Diag::ParseLong(char *argv, long &value) +otError Diag::ParseLong(char *aArgVector, long &aValue) { char *endptr; - value = strtol(argv, &endptr, 0); + aValue = strtol(aArgVector, &endptr, 0); return (*endptr == '\0') ? OT_ERROR_NONE : OT_ERROR_PARSE; } -void Diag::TxPacket() +void Diag::TxPacket(void) { sTxPacket->mLength = sTxLen; sTxPacket->mChannel = sChannel; @@ -190,142 +178,150 @@ void Diag::TxPacket() { sTxPacket->mPsdu[i] = i; } - otPlatRadioTransmit(sContext, sTxPacket); + + otPlatRadioTransmit(sInstance, sTxPacket); } -void Diag::ProcessChannel(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen) +void Diag::ProcessChannel(int aArgCount, char *aArgVector[]) { otError error = OT_ERROR_NONE; + VerifyOrExit(otPlatDiagModeGet(), error = OT_ERROR_INVALID_STATE); - if (argc == 0) + if (aArgCount == 0) { - snprintf(aOutput, aOutputMaxLen, "channel: %d\r\n", sChannel); + snprintf(sOutput, sizeof(sOutput), "channel: %d\r\n", sChannel); } else { long value; - SuccessOrExit(error = ParseLong(argv[0], value)); + SuccessOrExit(error = ParseLong(aArgVector[0], value)); VerifyOrExit(value >= OT_RADIO_CHANNEL_MIN && value <= OT_RADIO_CHANNEL_MAX, error = OT_ERROR_INVALID_ARGS); - sChannel = static_cast(value); - // listen on the set channel immediately - otPlatRadioReceive(sContext, sChannel); + sChannel = static_cast(value); + otPlatRadioReceive(sInstance, sChannel); otPlatDiagChannelSet(sChannel); - snprintf(aOutput, aOutputMaxLen, "set channel to %d\r\nstatus 0x%02x\r\n", sChannel, error); + + snprintf(sOutput, sizeof(sOutput), "set channel to %d\r\nstatus 0x%02x\r\n", sChannel, error); } exit: - AppendErrorResult(error, aOutput, aOutputMaxLen); + AppendErrorResult(error); } -void Diag::ProcessPower(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen) +void Diag::ProcessPower(int aArgCount, char *aArgVector[]) { otError error = OT_ERROR_NONE; VerifyOrExit(otPlatDiagModeGet(), error = OT_ERROR_INVALID_STATE); - if (argc == 0) + if (aArgCount == 0) { - snprintf(aOutput, aOutputMaxLen, "tx power: %d dBm\r\n", sTxPower); + snprintf(sOutput, sizeof(sOutput), "tx power: %d dBm\r\n", sTxPower); } else { long value; - SuccessOrExit(error = ParseLong(argv[0], value)); + SuccessOrExit(error = ParseLong(aArgVector[0], value)); + sTxPower = static_cast(value); otPlatDiagTxPowerSet(sTxPower); - snprintf(aOutput, aOutputMaxLen, "set tx power to %d dBm\r\nstatus 0x%02x\r\n", sTxPower, error); + + snprintf(sOutput, sizeof(sOutput), "set tx power to %d dBm\r\nstatus 0x%02x\r\n", sTxPower, error); } exit: - AppendErrorResult(error, aOutput, aOutputMaxLen); + AppendErrorResult(error); } -void Diag::ProcessSend(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen) +void Diag::ProcessSend(int aArgCount, char *aArgVector[]) { otError error = OT_ERROR_NONE; long value; VerifyOrExit(otPlatDiagModeGet(), error = OT_ERROR_INVALID_STATE); - VerifyOrExit(argc == 2, error = OT_ERROR_INVALID_ARGS); + VerifyOrExit(aArgCount == 2, error = OT_ERROR_INVALID_ARGS); - SuccessOrExit(error = ParseLong(argv[0], value)); + SuccessOrExit(error = ParseLong(aArgVector[0], value)); sTxPackets = static_cast(value); - SuccessOrExit(error = ParseLong(argv[1], value)); + SuccessOrExit(error = ParseLong(aArgVector[1], value)); VerifyOrExit(value <= OT_RADIO_FRAME_MAX_SIZE, error = OT_ERROR_INVALID_ARGS); sTxLen = static_cast(value); - snprintf(aOutput, aOutputMaxLen, "sending %#x packet(s), length %#x\r\nstatus 0x%02x\r\n", static_cast(sTxPackets), static_cast(sTxLen), error); + snprintf(sOutput, sizeof(sOutput), "sending %#x packet(s), length %#x\r\nstatus 0x%02x\r\n", + static_cast(sTxPackets), static_cast(sTxLen), error); TxPacket(); exit: - AppendErrorResult(error, aOutput, aOutputMaxLen); + AppendErrorResult(error); } -void Diag::ProcessRepeat(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen) +void Diag::ProcessRepeat(int aArgCount, char *aArgVector[]) { otError error = OT_ERROR_NONE; VerifyOrExit(otPlatDiagModeGet(), error = OT_ERROR_INVALID_STATE); - VerifyOrExit(argc > 0, error = OT_ERROR_INVALID_ARGS); + VerifyOrExit(aArgCount > 0, error = OT_ERROR_INVALID_ARGS); - if (strcmp(argv[0], "stop") == 0) + if (strcmp(aArgVector[0], "stop") == 0) { - otPlatAlarmMilliStop(sContext); + otPlatAlarmMilliStop(sInstance); sRepeatActive = false; - snprintf(aOutput, aOutputMaxLen, "repeated packet transmission is stopped\r\nstatus 0x%02x\r\n", error); + snprintf(sOutput, sizeof(sOutput), "repeated packet transmission is stopped\r\nstatus 0x%02x\r\n", error); } else { long value; - VerifyOrExit(argc == 2, error = OT_ERROR_INVALID_ARGS); + VerifyOrExit(aArgCount == 2, error = OT_ERROR_INVALID_ARGS); - SuccessOrExit(error = ParseLong(argv[0], value)); + SuccessOrExit(error = ParseLong(aArgVector[0], value)); sTxPeriod = static_cast(value); - SuccessOrExit(error = ParseLong(argv[1], value)); + SuccessOrExit(error = ParseLong(aArgVector[1], value)); VerifyOrExit(value <= OT_RADIO_FRAME_MAX_SIZE, error = OT_ERROR_INVALID_ARGS); sTxLen = static_cast(value); sRepeatActive = true; uint32_t now = otPlatAlarmMilliGetNow(); - otPlatAlarmMilliStartAt(sContext, now, sTxPeriod); - snprintf(aOutput, aOutputMaxLen, "sending packets of length %#x at the delay of %#x ms\r\nstatus 0x%02x\r\n", static_cast(sTxLen), static_cast(sTxPeriod), error); + otPlatAlarmMilliStartAt(sInstance, now, sTxPeriod); + snprintf(sOutput, sizeof(sOutput), "sending packets of length %#x at the delay of %#x ms\r\nstatus 0x%02x\r\n", + static_cast(sTxLen), static_cast(sTxPeriod), error); } exit: - AppendErrorResult(error, aOutput, aOutputMaxLen); + AppendErrorResult(error); } -void Diag::ProcessStats(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen) +void Diag::ProcessStats(int aArgCount, char *aArgVector[]) { otError error = OT_ERROR_NONE; + OT_UNUSED_VARIABLE(aArgCount); + OT_UNUSED_VARIABLE(aArgVector); + VerifyOrExit(otPlatDiagModeGet(), error = OT_ERROR_INVALID_STATE); - snprintf(aOutput, aOutputMaxLen, "received packets: %d\r\nsent packets: %d\r\nfirst received packet: rssi=%d, lqi=%d\r\n", - static_cast(sStats.received_packets), static_cast(sStats.sent_packets), - static_cast(sStats.first_rssi), static_cast(sStats.first_lqi)); + snprintf(sOutput, sizeof(sOutput), + "received packets: %d\r\nsent packets: %d\r\nfirst received packet: rssi=%d, lqi=%d\r\n", + static_cast(sStats.mReceivedPackets), static_cast(sStats.mSentPackets), + static_cast(sStats.mFirstRssi), static_cast(sStats.mFirstLqi)); exit: - OT_UNUSED_VARIABLE(argc); - OT_UNUSED_VARIABLE(argv); - AppendErrorResult(error, aOutput, aOutputMaxLen); + AppendErrorResult(error); } void Diag::DiagTransmitDone(otInstance *aInstance, otError aError) { - VerifyOrExit(aInstance == sContext); + VerifyOrExit(aInstance == sInstance); if (aError == OT_ERROR_NONE) { - sStats.sent_packets++; + sStats.mSentPackets++; if (sTxPackets > 1) { @@ -344,18 +340,18 @@ exit: void Diag::DiagReceiveDone(otInstance *aInstance, otRadioFrame *aFrame, otError aError) { - VerifyOrExit(aInstance == sContext); + VerifyOrExit(aInstance == sInstance); if (aError == OT_ERROR_NONE) { // for sensitivity test, only record the rssi and lqi for the first packet - if (sStats.received_packets == 0) + if (sStats.mReceivedPackets == 0) { - sStats.first_rssi = aFrame->mRssi; - sStats.first_lqi = aFrame->mLqi; + sStats.mFirstRssi = aFrame->mRssi; + sStats.mFirstLqi = aFrame->mLqi; } - sStats.received_packets++; + sStats.mReceivedPackets++; } otPlatDiagRadioReceived(aInstance, aFrame, aError); @@ -365,7 +361,7 @@ exit: void Diag::AlarmFired(otInstance *aInstance) { - VerifyOrExit(aInstance == sContext); + VerifyOrExit(aInstance == sInstance); if(sRepeatActive) { diff --git a/src/diag/diag_process.hpp b/src/diag/diag_process.hpp index 8e1c911b4..cf7d671cd 100644 --- a/src/diag/diag_process.hpp +++ b/src/diag/diag_process.hpp @@ -47,46 +47,50 @@ namespace ot { namespace Diagnostics { -#define MAX_DIAG_OUTPUT 256 - -struct Command -{ - const char *mName; ///< A pointer to the command string. - void (*mCommand)(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); ///< A function pointer to process the command. -}; - -struct DiagStats -{ - uint32_t received_packets; - uint32_t sent_packets; - int8_t first_rssi; - uint8_t first_lqi; -}; - class Diag { public: static void Init(otInstance *aInstance); - static char *ProcessCmd(int argc, char *argv[]); - static bool isEnabled(void); + static const char *ProcessCmd(int aArgCount, char *aArgVector[]); + static bool IsEnabled(void); static void DiagTransmitDone(otInstance *aInstance, otError aError); static void DiagReceiveDone(otInstance *aInstance, otRadioFrame *aFrame, otError aError); static void AlarmFired(otInstance *aInstance); private: - static void AppendErrorResult(otError error, char *aOutput, size_t aOutputMaxLen); - static void ProcessStart(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); - static void ProcessStop(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); - static void ProcessSend(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); - static void ProcessRepeat(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); - static void ProcessStats(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); - static void ProcessChannel(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); - static void ProcessPower(int argc, char *argv[], char *aOutput, size_t aOutputMaxLen); + + enum + { + kMaxOutputSize = 256, + }; + + struct DiagStats + { + uint32_t mReceivedPackets; + uint32_t mSentPackets; + int8_t mFirstRssi; + uint8_t mFirstLqi; + }; + + struct Command + { + const char *mName; + void (*mHandler)(int aArgCount, char *aArgVector[]); + }; + + static void AppendErrorResult(otError aError); + static void ProcessStart(int aArgCount, char *aArgVector[]); + static void ProcessStop(int aArgCount, char *aArgVector[]); + static void ProcessSend(int aArgCount, char *aArgVector[]); + static void ProcessRepeat(int aArgCount, char *aArgVector[]); + static void ProcessStats(int aArgCount, char *aArgVector[]); + static void ProcessChannel(int aArgCount, char *aArgVector[]); + static void ProcessPower(int aArgCount, char *aArgVector[]); static void TxPacket(void); static otError ParseLong(char *aString, long &aLong); - static char sDiagOutput[]; + static char sOutput[]; static const struct Command sCommands[]; static struct DiagStats sStats; static int8_t sTxPower; @@ -95,7 +99,7 @@ private: static uint32_t sTxPeriod; static uint32_t sTxPackets; static otRadioFrame *sTxPacket; - static otInstance *sContext; + static otInstance *sInstance; static bool sRepeatActive; }; diff --git a/src/diag/openthread-diag.cpp b/src/diag/openthread-diag.cpp index 8033f2352..741d86b5c 100644 --- a/src/diag/openthread-diag.cpp +++ b/src/diag/openthread-diag.cpp @@ -40,6 +40,7 @@ #include #include "diag_process.hpp" +#include "common/code_utils.hpp" using namespace ot::Diagnostics; @@ -48,37 +49,96 @@ void otDiagInit(otInstance *aInstance) Diag::Init(aInstance); } -char *otDiagProcessCmd(int aArgc, char *aArgv[]) +const char *otDiagProcessCmd(int aArgCount, char *aArgVector[]) { - return Diag::ProcessCmd(aArgc, aArgv); + return Diag::ProcessCmd(aArgCount, aArgVector); } -char *otDiagProcessCmdLine(char *aString) +static bool IsSpace(char aChar) { - char *argv[8]; - int argc = 0; - int length = static_cast(strlen(aString)); - char *cmd; + return (aChar == ' ') || (aChar == '\t'); +} - for (; *aString == ' '; aString++, length--); +static bool IsNullOrNewline(char aChar) +{ + return (aChar == 0) || (aChar == '\n') || (aChar == '\r'); +} - for (cmd = aString + 1; (cmd < aString + length) && (cmd != NULL); ++cmd) +const char *otDiagProcessCmdLine(const char *aInput) +{ + enum { - if (*cmd == ' ' || *cmd == '\r' || *cmd == '\n') + kMaxArgs = 32, + kMaxCommandBuffer = 256, + }; + + otError error = OT_ERROR_NONE; + char buffer[kMaxCommandBuffer]; + char *argVector[kMaxArgs]; + int argCount = 0; + char *bufPtr = &buffer[0]; + uint16_t bufLen = sizeof(buffer); + const char *output = "\r\n"; + + while (!IsNullOrNewline(*aInput)) + { + while (IsSpace(*aInput)) { - *cmd = '\0'; + aInput++; } - if (*(cmd - 1) == '\0' && *cmd != ' ') + argVector[argCount] = bufPtr; + + while (!IsSpace(*aInput) && !IsNullOrNewline(*aInput)) { - argv[argc++] = cmd; + *bufPtr++ = *aInput++; + VerifyOrExit(--bufLen > 0, error = OT_ERROR_NO_BUFS); + } + + if (argVector[argCount] != bufPtr) + { + *bufPtr++ = 0; + VerifyOrExit(--bufLen > 0, error = OT_ERROR_NO_BUFS); + + argCount++; + VerifyOrExit(argCount < kMaxArgs, error = OT_ERROR_INVALID_ARGS); } } - return Diag::ProcessCmd(argc, argv); +exit: + + switch (error) + { + case OT_ERROR_NONE: + + if (argCount >= 1) + { + output = Diag::ProcessCmd(argCount - 1, (argCount == 1) ? NULL : &argVector[1]); + } + else + { + output = Diag::ProcessCmd(0, NULL); + } + + break; + + case OT_ERROR_NO_BUFS: + output = "failed: command string too long\r\n"; + break; + + case OT_ERROR_INVALID_ARGS: + output = "failed: command string contains too many arguments\r\n"; + break; + + default: + output = "failed to parse command string\n\r"; + break; + } + + return output; } -bool otDiagIsEnabled() +bool otDiagIsEnabled(void) { - return Diag::isEnabled(); + return Diag::IsEnabled(); } diff --git a/src/ncp/ncp_base_mtd.cpp b/src/ncp/ncp_base_mtd.cpp index 0dc151b52..12b1964a5 100644 --- a/src/ncp/ncp_base_mtd.cpp +++ b/src/ncp/ncp_base_mtd.cpp @@ -2145,15 +2145,14 @@ exit: otError NcpBase::SetPropertyHandler_NEST_STREAM_MFG(uint8_t aHeader) { const char *string = NULL; - char *output = NULL; + const char *output = NULL; otError error = OT_ERROR_NONE; error = mDecoder.ReadUtf8(string); VerifyOrExit(error == OT_ERROR_NONE, error = WriteLastStatusFrame(aHeader, ThreadErrorToSpinelStatus(error))); - // All diagnostics related features are processed within diagnostics module - output = otDiagProcessCmdLine(const_cast(string)); + output = otDiagProcessCmdLine(string); // Prepare the response SuccessOrExit(error = mEncoder.BeginFrame(aHeader, SPINEL_CMD_PROP_VALUE_IS, SPINEL_PROP_NEST_STREAM_MFG)); diff --git a/tests/unit/test_diag.cpp b/tests/unit/test_diag.cpp index 38dc2bc4f..bb315b90d 100644 --- a/tests/unit/test_diag.cpp +++ b/tests/unit/test_diag.cpp @@ -79,18 +79,83 @@ extern "C" void otPlatRadioTxStarted(otInstance *, otRadioFrame *aFrame) (void) aFrame; } - /** - * diagnostics module tests + * Converts a given string replacing '\n', '\r', '\t', etc. with literal strings "\n", "\r", "\t", etc. to make it + * printable on console. + * */ -void TestDiag() +const char *MakePrintable(const char *aString) { - static const struct + static char printableString[256]; + + char *ptr = &printableString[0]; + + *ptr++ = '\"'; + + while ((*aString != 0) && (ptr < &printableString[sizeof(printableString) - 4])) { - const char *command; - const char *output; - } tests[] = + // We ensure that at least 4 characters are remaining in output, + // two for encoding the current char (in case it needs a backslash) + // and two for the ending `"` and null char. + + char c = *aString++; + bool addBackslash = true; + + switch (c) + { + case '\n': + c = 'n'; + break; + + case '\r': + c = 'r'; + break; + + case '\t': + c = 't'; + break; + + case '\"': + c = '\"'; + break; + + case '\'': + c = '\''; + break; + + case '\\': + break; + + default: + addBackslash = false; + break; + } + + if (addBackslash) + { + *ptr++ = '\\'; + } + + *ptr++ = c; + } + + *ptr++ = '\"'; + *ptr = 0; + + return printableString; +} + +void TestDiag(void) +{ + struct TestCommand { + const char *mCommand; + const char *mExpectedOutput; + }; + + static const TestCommand tests[] = + { + { "diag\n", "diagnostics mode is disabled\r\n", @@ -104,7 +169,11 @@ void TestDiag() "start diagnostics mode\r\nstatus 0x00\r\n", }, { - "diag\n", + "diag", + "diagnostics mode is enabled\r\n", + }, + { + "", "diagnostics mode is enabled\r\n", }, { @@ -135,6 +204,10 @@ void TestDiag() "diag send 20 100\n", "sending 0x14 packet(s), length 0x64\r\nstatus 0x00\r\n", }, + { + " diag \t send \t 20\t100", // Check parsing of extra space chars between args + "sending 0x14 packet(s), length 0x64\r\nstatus 0x00\r\n", + }, { "diag repeat 100 100\n", "sending packets of length 0x64 at the delay of 0x64 ms\r\nstatus 0x00\r\n" @@ -144,9 +217,17 @@ void TestDiag() "received packets: 0\r\nsent packets: 0\r\nfirst received packet: rssi=0, lqi=0\r\n\nstop diagnostics mode\r\nstatus 0x00\r\n", }, { - "diag\n", + "diag", "diagnostics mode is disabled\r\n", }, + { + "diag 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32", + "failed: command string contains too many arguments\r\n", + }, + { + NULL, + NULL, + } }; // initialize platform layer @@ -157,19 +238,17 @@ void TestDiag() // initialize diagnostics module otDiagInit(NULL); - // test diagnostics commands - VerifyOrQuit(!otDiagIsEnabled(), "diagnostics mode shoud be disabled as default\n"); + VerifyOrQuit(!otDiagIsEnabled(), "diagnostics mode should be disabled as default\n"); - for (unsigned int i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) + for (const TestCommand *test = &tests[0]; test->mCommand != NULL; test++) { - char string[50]; - char *output = NULL; + const char *output = NULL; - memcpy(string, tests[i].command, strlen(tests[i].command) + 1); + printf("\nCommand: %s", MakePrintable(test->mCommand)); + output = otDiagProcessCmdLine(test->mCommand); + printf("\nOutput: %s\n", MakePrintable(output)); - output = otDiagProcessCmdLine(string); - VerifyOrQuit(memcmp(output, tests[i].output, strlen(tests[i].output)) == 0, - "Test Diagnostics module failed\r\n"); + VerifyOrQuit(strcmp(output, test->mExpectedOutput) == 0, "diagnostics output does not match expected result\n"); } } @@ -177,7 +256,7 @@ void TestDiag() int main(void) { TestDiag(); - printf("All tests passed\n"); + printf("\nAll tests passed\n"); return 0; } #endif