diff --git a/include/openthread/diag.h b/include/openthread/diag.h index 87a61e7cd..35ecc765e 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] 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. + * @param[in] aArgCount The argument counter of diagnostics command line. + * @param[in] aArgVector The argument vector of diagnostics command line. + * @param[out] aOutput The diagnostics execution result. + * @param[in] aOutputMaxLen The output buffer size. * */ -const char *otDiagProcessCmd(int aArgCount, char *aArgVector[]); +void otDiagProcessCmd(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); /** * This function processes a factory diagnostics command line. * - * @param[in] aString A NULL-terminated input string. - * - * @returns A pointer to the output string. + * @param[in] aString A NULL-terminated input string. + * @param[out] aOutput The diagnostics execution result. + * @param[in] aOutputMaxLen The output buffer size. * */ -const char *otDiagProcessCmdLine(const char *aString); +void otDiagProcessCmdLine(const char *aString, char *aOutput, size_t aOutputMaxLen); /** * This function indicates whether or not the factory diagnostics mode is enabled. diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index 510f3cb75..74e20e40f 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -3466,8 +3466,12 @@ exit: #if OPENTHREAD_ENABLE_DIAG void Interpreter::ProcessDiag(int argc, char *argv[]) { + char output[OPENTHREAD_CONFIG_DIAG_OUTPUT_BUFFER_SIZE]; + // all diagnostics related features are processed within diagnostics module - mServer->OutputFormat("%s\r\n", otDiagProcessCmd(argc, argv)); + output[sizeof(output) - 1] = '\0'; + otDiagProcessCmd(argc, argv, output, sizeof(output) - 1); + mServer->OutputFormat("%s\n", output); } #endif diff --git a/src/diag/diag_process.cpp b/src/diag/diag_process.cpp index 886fff991..4fe5af068 100644 --- a/src/diag/diag_process.cpp +++ b/src/diag/diag_process.cpp @@ -56,7 +56,6 @@ const struct Diag::Command Diag::sCommands[] = { NULL, NULL }, }; -char Diag::sOutput[Diag::kMaxOutputSize]; struct Diag::DiagStats Diag::sStats; int8_t Diag::sTxPower; @@ -83,11 +82,11 @@ void Diag::Init(otInstance *aInstance) otPlatDiagTxPowerSet(sTxPower); } -const char *Diag::ProcessCmd(int aArgCount, char *aArgVector[]) +void Diag::ProcessCmd(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { if (aArgCount == 0) { - snprintf(sOutput, sizeof(sOutput), "diagnostics mode is %s\r\n", otPlatDiagModeGet() ? "enabled" : "disabled"); + snprintf(aOutput, aOutputMaxLen, "diagnostics mode is %s\r\n", otPlatDiagModeGet() ? "enabled" : "disabled"); ExitNow(); } @@ -95,16 +94,16 @@ const char *Diag::ProcessCmd(int aArgCount, char *aArgVector[]) { if (strcmp(aArgVector[0], command->mName) == 0) { - command->mHandler(aArgCount - 1, (aArgCount > 1) ? &aArgVector[1] : NULL); + command->mHandler(aArgCount - 1, (aArgCount > 1) ? &aArgVector[1] : NULL, aOutput, aOutputMaxLen); ExitNow(); } } // more platform specific features will be processed under platform layer - otPlatDiagProcess(sInstance, aArgCount, aArgVector, sOutput, sizeof(sOutput)); + otPlatDiagProcess(sInstance, aArgCount, aArgVector, aOutput, aOutputMaxLen); exit: - return sOutput; + return; } bool Diag::IsEnabled(void) @@ -112,15 +111,15 @@ bool Diag::IsEnabled(void) return otPlatDiagModeGet(); } -void Diag::AppendErrorResult(otError aError) +void Diag::AppendErrorResult(otError aError, char *aOutput, size_t aOutputMaxLen) { if (aError != OT_ERROR_NONE) { - snprintf(sOutput, sizeof(sOutput), "failed\r\nstatus %#x\r\n", aError); + snprintf(aOutput, aOutputMaxLen, "failed\r\nstatus %#x\r\n", aError); } } -void Diag::ProcessStart(int aArgCount, char *aArgVector[]) +void Diag::ProcessStart(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { otError error = OT_ERROR_NONE; @@ -133,13 +132,13 @@ void Diag::ProcessStart(int aArgCount, char *aArgVector[]) SuccessOrExit(error = otPlatRadioReceive(sInstance, sChannel)); otPlatDiagModeSet(true); memset(&sStats, 0, sizeof(struct DiagStats)); - snprintf(sOutput, sizeof(sOutput), "start diagnostics mode\r\nstatus 0x%02x\r\n", error); + snprintf(aOutput, aOutputMaxLen, "start diagnostics mode\r\nstatus 0x%02x\r\n", error); exit: - AppendErrorResult(error); + AppendErrorResult(error, aOutput, aOutputMaxLen); } -void Diag::ProcessStop(int aArgCount, char *aArgVector[]) +void Diag::ProcessStop(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { otError error = OT_ERROR_NONE; @@ -152,14 +151,14 @@ void Diag::ProcessStop(int aArgCount, char *aArgVector[]) otPlatDiagModeSet(false); otPlatRadioSetPromiscuous(sInstance, false); - snprintf(sOutput, sizeof(sOutput), + 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.mReceivedPackets), static_cast(sStats.mSentPackets), static_cast(sStats.mFirstRssi), static_cast(sStats.mFirstLqi), error); exit: - AppendErrorResult(error); + AppendErrorResult(error, aOutput, aOutputMaxLen); } otError Diag::ParseLong(char *aArgVector, long &aValue) @@ -182,7 +181,7 @@ void Diag::TxPacket(void) otPlatRadioTransmit(sInstance, sTxPacket); } -void Diag::ProcessChannel(int aArgCount, char *aArgVector[]) +void Diag::ProcessChannel(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { otError error = OT_ERROR_NONE; @@ -191,7 +190,7 @@ void Diag::ProcessChannel(int aArgCount, char *aArgVector[]) if (aArgCount == 0) { - snprintf(sOutput, sizeof(sOutput), "channel: %d\r\n", sChannel); + snprintf(aOutput, aOutputMaxLen, "channel: %d\r\n", sChannel); } else { @@ -204,14 +203,14 @@ void Diag::ProcessChannel(int aArgCount, char *aArgVector[]) otPlatRadioReceive(sInstance, sChannel); otPlatDiagChannelSet(sChannel); - snprintf(sOutput, sizeof(sOutput), "set channel to %d\r\nstatus 0x%02x\r\n", sChannel, error); + snprintf(aOutput, aOutputMaxLen, "set channel to %d\r\nstatus 0x%02x\r\n", sChannel, error); } exit: - AppendErrorResult(error); + AppendErrorResult(error, aOutput, aOutputMaxLen); } -void Diag::ProcessPower(int aArgCount, char *aArgVector[]) +void Diag::ProcessPower(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { otError error = OT_ERROR_NONE; @@ -219,7 +218,7 @@ void Diag::ProcessPower(int aArgCount, char *aArgVector[]) if (aArgCount == 0) { - snprintf(sOutput, sizeof(sOutput), "tx power: %d dBm\r\n", sTxPower); + snprintf(aOutput, aOutputMaxLen, "tx power: %d dBm\r\n", sTxPower); } else { @@ -230,14 +229,14 @@ void Diag::ProcessPower(int aArgCount, char *aArgVector[]) sTxPower = static_cast(value); otPlatDiagTxPowerSet(sTxPower); - snprintf(sOutput, sizeof(sOutput), "set tx power to %d dBm\r\nstatus 0x%02x\r\n", sTxPower, error); + snprintf(aOutput, aOutputMaxLen, "set tx power to %d dBm\r\nstatus 0x%02x\r\n", sTxPower, error); } exit: - AppendErrorResult(error); + AppendErrorResult(error, aOutput, aOutputMaxLen); } -void Diag::ProcessSend(int aArgCount, char *aArgVector[]) +void Diag::ProcessSend(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { otError error = OT_ERROR_NONE; long value; @@ -252,15 +251,15 @@ void Diag::ProcessSend(int aArgCount, char *aArgVector[]) VerifyOrExit(value <= OT_RADIO_FRAME_MAX_SIZE, error = OT_ERROR_INVALID_ARGS); sTxLen = static_cast(value); - snprintf(sOutput, sizeof(sOutput), "sending %#x packet(s), length %#x\r\nstatus 0x%02x\r\n", + snprintf(aOutput, aOutputMaxLen, "sending %#x packet(s), length %#x\r\nstatus 0x%02x\r\n", static_cast(sTxPackets), static_cast(sTxLen), error); TxPacket(); exit: - AppendErrorResult(error); + AppendErrorResult(error, aOutput, aOutputMaxLen); } -void Diag::ProcessRepeat(int aArgCount, char *aArgVector[]) +void Diag::ProcessRepeat(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { otError error = OT_ERROR_NONE; @@ -271,7 +270,7 @@ void Diag::ProcessRepeat(int aArgCount, char *aArgVector[]) { otPlatAlarmMilliStop(sInstance); sRepeatActive = false; - snprintf(sOutput, sizeof(sOutput), "repeated packet transmission is stopped\r\nstatus 0x%02x\r\n", error); + snprintf(aOutput, aOutputMaxLen, "repeated packet transmission is stopped\r\nstatus 0x%02x\r\n", error); } else { @@ -289,15 +288,15 @@ void Diag::ProcessRepeat(int aArgCount, char *aArgVector[]) sRepeatActive = true; uint32_t now = otPlatAlarmMilliGetNow(); 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", + 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); } exit: - AppendErrorResult(error); + AppendErrorResult(error, aOutput, aOutputMaxLen); } -void Diag::ProcessStats(int aArgCount, char *aArgVector[]) +void Diag::ProcessStats(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { otError error = OT_ERROR_NONE; @@ -306,13 +305,13 @@ void Diag::ProcessStats(int aArgCount, char *aArgVector[]) VerifyOrExit(otPlatDiagModeGet(), error = OT_ERROR_INVALID_STATE); - snprintf(sOutput, sizeof(sOutput), + snprintf(aOutput, aOutputMaxLen, "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: - AppendErrorResult(error); + AppendErrorResult(error, aOutput, aOutputMaxLen); } void Diag::DiagTransmitDone(otInstance *aInstance, otError aError) diff --git a/src/diag/diag_process.hpp b/src/diag/diag_process.hpp index c304193d3..8f44bd1dd 100644 --- a/src/diag/diag_process.hpp +++ b/src/diag/diag_process.hpp @@ -50,7 +50,7 @@ class Diag { public: static void Init(otInstance *aInstance); - static const char *ProcessCmd(int aArgCount, char *aArgVector[]); + static void ProcessCmd(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); static bool IsEnabled(void); static void DiagTransmitDone(otInstance *aInstance, otError aError); @@ -58,12 +58,6 @@ public: static void AlarmFired(otInstance *aInstance); private: - - enum - { - kMaxOutputSize = OPENTHREAD_CONFIG_DIAG_OUTPUT_BUFFER_SIZE, - }; - struct DiagStats { uint32_t mReceivedPackets; @@ -75,21 +69,20 @@ private: struct Command { const char *mName; - void (*mHandler)(int aArgCount, char *aArgVector[]); + void (*mHandler)(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); }; - 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 AppendErrorResult(otError aError, char *aOutput, size_t aOutputMaxLen); + static void ProcessStart(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); + static void ProcessStop(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); + static void ProcessSend(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); + static void ProcessRepeat(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); + static void ProcessStats(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); + static void ProcessChannel(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); + static void ProcessPower(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen); static void TxPacket(void); static otError ParseLong(char *aString, long &aLong); - static char sOutput[]; static const struct Command sCommands[]; static struct DiagStats sStats; static int8_t sTxPower; diff --git a/src/diag/openthread-diag.cpp b/src/diag/openthread-diag.cpp index 3f4350ac6..13f4fdaf0 100644 --- a/src/diag/openthread-diag.cpp +++ b/src/diag/openthread-diag.cpp @@ -49,9 +49,9 @@ void otDiagInit(otInstance *aInstance) Diag::Init(aInstance); } -const char *otDiagProcessCmd(int aArgCount, char *aArgVector[]) +void otDiagProcessCmd(int aArgCount, char *aArgVector[], char *aOutput, size_t aOutputMaxLen) { - return Diag::ProcessCmd(aArgCount, aArgVector); + Diag::ProcessCmd(aArgCount, aArgVector, aOutput, aOutputMaxLen); } static bool IsSpace(char aChar) @@ -64,7 +64,7 @@ static bool IsNullOrNewline(char aChar) return (aChar == 0) || (aChar == '\n') || (aChar == '\r'); } -const char *otDiagProcessCmdLine(const char *aInput) +void otDiagProcessCmdLine(const char *aInput, char *aOutput, size_t aOutputMaxLen) { enum { @@ -78,7 +78,6 @@ const char *otDiagProcessCmdLine(const char *aInput) int argCount = 0; char *bufPtr = &buffer[0]; uint16_t bufLen = sizeof(buffer); - const char *output = "\r\n"; while (!IsNullOrNewline(*aInput)) { @@ -113,29 +112,27 @@ exit: if (argCount >= 1) { - output = Diag::ProcessCmd(argCount - 1, (argCount == 1) ? NULL : &argVector[1]); + Diag::ProcessCmd(argCount - 1, (argCount == 1) ? NULL : &argVector[1], aOutput, aOutputMaxLen); } else { - output = Diag::ProcessCmd(0, NULL); + Diag::ProcessCmd(0, NULL, aOutput, aOutputMaxLen); } break; case OT_ERROR_NO_BUFS: - output = "failed: command string too long\r\n"; + snprintf(aOutput, aOutputMaxLen, "failed: command string too long\r\n"); break; case OT_ERROR_INVALID_ARGS: - output = "failed: command string contains too many arguments\r\n"; + snprintf(aOutput, aOutputMaxLen, "failed: command string contains too many arguments\r\n"); break; default: - output = "failed to parse command string\n\r"; + snprintf(aOutput, aOutputMaxLen, "failed to parse command string\r\n"); break; } - - return output; } bool otDiagIsEnabled(void) diff --git a/src/ncp/ncp_base.cpp b/src/ncp/ncp_base.cpp index 5b955dada..af636c446 100644 --- a/src/ncp/ncp_base.cpp +++ b/src/ncp/ncp_base.cpp @@ -1306,14 +1306,15 @@ exit: otError NcpBase::HandlePropertySet_SPINEL_PROP_NEST_STREAM_MFG(uint8_t aHeader) { const char *string = NULL; - const char *output = NULL; - otError error = OT_ERROR_NONE; + char output[OPENTHREAD_CONFIG_DIAG_OUTPUT_BUFFER_SIZE]; + otError error = OT_ERROR_NONE; error = mDecoder.ReadUtf8(string); VerifyOrExit(error == OT_ERROR_NONE, error = WriteLastStatusFrame(aHeader, ThreadErrorToSpinelStatus(error))); - output = otDiagProcessCmdLine(string); + output[sizeof(output) - 1] = '\0'; + otDiagProcessCmdLine(string, output, sizeof(output) - 1); // 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 151a65155..80b75b3cc 100644 --- a/tests/unit/test_diag.cpp +++ b/tests/unit/test_diag.cpp @@ -243,10 +243,11 @@ void TestDiag(void) for (const TestCommand *test = &tests[0]; test->mCommand != NULL; test++) { - const char *output = NULL; + char output[OPENTHREAD_CONFIG_DIAG_OUTPUT_BUFFER_SIZE]; printf("\nCommand: %s", MakePrintable(test->mCommand)); - output = otDiagProcessCmdLine(test->mCommand); + output[sizeof(output) - 1] = '\0'; + otDiagProcessCmdLine(test->mCommand, output, sizeof(output) - 1); printf("\nOutput: %s\n", MakePrintable(output)); VerifyOrQuit(strcmp(output, test->mExpectedOutput) == 0, "diagnostics output does not match expected result\n");