From 9cb48245e5b0ff972afb6968dd655ecca0759b63 Mon Sep 17 00:00:00 2001 From: Robert Lubos Date: Wed, 23 Jan 2019 01:07:46 +0100 Subject: [PATCH] [cli] fix GetOwner function for console cli backend (#3486) Current implementation of GetOwner function for single-instance CLI assumed that `cli_uart` backend is used. This caused application crash when different backend was used and `cli_uart` was not initialized (and in result sUartServer static pointer was not set). This commit fixes the issue by moving static CLI Server pointer and the mInterpreter field to the to the base Server class and providing getter method to obtain the Interpreter instance. With this approach, any backend derived from the Server class will be able to initialize the static pointer and will provide a valid reference to the Interpreter class instance. Signed-off-by: Robert Lubos --- Android.mk | 1 + .../libopenthread-cli-windows.vcxproj | 1 + .../libopenthread-cli-windows.vcxproj.filters | 3 ++ etc/visual-studio/libopenthread-cli.vcxproj | 1 + .../libopenthread-cli.vcxproj.filters | 5 ++- src/cli/Makefile.am | 1 + src/cli/cli.cpp | 2 +- src/cli/cli.hpp | 2 +- src/cli/cli_coap.cpp | 1 + src/cli/cli_coap_secure.cpp | 1 + src/cli/cli_console.cpp | 14 +++---- src/cli/cli_console.hpp | 2 - src/cli/cli_dataset.cpp | 1 + src/cli/cli_dataset.hpp | 2 - src/cli/cli_instance.cpp | 1 + src/cli/cli_server.cpp | 42 +++++++++++++++++++ src/cli/cli_server.hpp | 21 ++++++++++ src/cli/cli_uart.cpp | 26 ++++++------ src/cli/cli_uart.hpp | 12 ------ src/cli/cli_udp.cpp | 1 + 20 files changed, 99 insertions(+), 41 deletions(-) create mode 100644 src/cli/cli_server.cpp diff --git a/Android.mk b/Android.mk index 19bd1abfd..d90560614 100644 --- a/Android.mk +++ b/Android.mk @@ -282,6 +282,7 @@ LOCAL_SRC_FILES := \ src/cli/cli_coap.cpp \ src/cli/cli_console.cpp \ src/cli/cli_dataset.cpp \ + src/cli/cli_server.cpp \ src/cli/cli_uart.cpp \ src/cli/cli_udp.cpp \ src/posix/main.c \ diff --git a/etc/visual-studio/libopenthread-cli-windows.vcxproj b/etc/visual-studio/libopenthread-cli-windows.vcxproj index db367ff02..90023b00b 100644 --- a/etc/visual-studio/libopenthread-cli-windows.vcxproj +++ b/etc/visual-studio/libopenthread-cli-windows.vcxproj @@ -60,6 +60,7 @@ + diff --git a/etc/visual-studio/libopenthread-cli-windows.vcxproj.filters b/etc/visual-studio/libopenthread-cli-windows.vcxproj.filters index a5869353a..9bd264429 100644 --- a/etc/visual-studio/libopenthread-cli-windows.vcxproj.filters +++ b/etc/visual-studio/libopenthread-cli-windows.vcxproj.filters @@ -16,6 +16,9 @@ Source Files + + Source Files + Source Files diff --git a/etc/visual-studio/libopenthread-cli.vcxproj b/etc/visual-studio/libopenthread-cli.vcxproj index 8120e9fc8..056570495 100644 --- a/etc/visual-studio/libopenthread-cli.vcxproj +++ b/etc/visual-studio/libopenthread-cli.vcxproj @@ -58,6 +58,7 @@ + diff --git a/etc/visual-studio/libopenthread-cli.vcxproj.filters b/etc/visual-studio/libopenthread-cli.vcxproj.filters index b9bb74f64..3e307f0a1 100644 --- a/etc/visual-studio/libopenthread-cli.vcxproj.filters +++ b/etc/visual-studio/libopenthread-cli.vcxproj.filters @@ -12,7 +12,10 @@ Source Files - + + + Source Files + Source Files diff --git a/src/cli/Makefile.am b/src/cli/Makefile.am index 4f6303500..880775087 100644 --- a/src/cli/Makefile.am +++ b/src/cli/Makefile.am @@ -147,6 +147,7 @@ SOURCES_COMMON = \ cli_coap_secure.cpp \ cli_console.cpp \ cli_dataset.cpp \ + cli_server.cpp \ cli_uart.cpp \ cli_udp.cpp \ $(NULL) diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index ee5904a03..d9064a7f0 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -3916,7 +3916,7 @@ Interpreter &Interpreter::GetOwner(OwnerLocator &aOwnerLocator) #else OT_UNUSED_VARIABLE(aOwnerLocator); - Interpreter &interpreter = Uart::sUartServer->GetInterpreter(); + Interpreter &interpreter = Server::sServer->GetInterpreter(); #endif return interpreter; } diff --git a/src/cli/cli.hpp b/src/cli/cli.hpp index 8f8c1bf65..c4348b291 100644 --- a/src/cli/cli.hpp +++ b/src/cli/cli.hpp @@ -43,7 +43,6 @@ #include #include "cli/cli_dataset.hpp" -#include "cli/cli_server.hpp" #include "cli/cli_udp.hpp" #if OPENTHREAD_ENABLE_APPLICATION_COAP @@ -82,6 +81,7 @@ namespace ot { namespace Cli { class Interpreter; +class Server; /** * This structure represents a CLI command. diff --git a/src/cli/cli_coap.cpp b/src/cli/cli_coap.cpp index d25227d14..beaed94ef 100644 --- a/src/cli/cli_coap.cpp +++ b/src/cli/cli_coap.cpp @@ -38,6 +38,7 @@ #include #include "cli/cli.hpp" +#include "cli/cli_server.hpp" #include "coap/coap_message.hpp" namespace ot { diff --git a/src/cli/cli_coap_secure.cpp b/src/cli/cli_coap_secure.cpp index 277b8ee76..e3e8c440d 100644 --- a/src/cli/cli_coap_secure.cpp +++ b/src/cli/cli_coap_secure.cpp @@ -39,6 +39,7 @@ #include #include "cli/cli.hpp" +#include "cli/cli_server.hpp" // header for place your x509 certificate and private key #include "x509_cert_key.hpp" diff --git a/src/cli/cli_console.cpp b/src/cli/cli_console.cpp index bfe25545c..f1e10620d 100644 --- a/src/cli/cli_console.cpp +++ b/src/cli/cli_console.cpp @@ -45,28 +45,26 @@ namespace ot { namespace Cli { -static Console *sServer; - static otDEFINE_ALIGNED_VAR(sCliConsoleRaw, sizeof(Console), uint64_t); extern "C" void otCliConsoleInit(otInstance *aInstance, otCliConsoleOutputCallback aCallback, void *aContext) { Instance *instance = static_cast(aInstance); - sServer = new (&sCliConsoleRaw) Console(instance); - sServer->SetOutputCallback(aCallback); - sServer->SetContext(aContext); + Server::sServer = new (&sCliConsoleRaw) Console(instance); + static_cast(Server::sServer)->SetOutputCallback(aCallback); + static_cast(Server::sServer)->SetContext(aContext); } extern "C" void otCliConsoleInputLine(char *aBuf, uint16_t aBufLength) { - sServer->ReceiveTask(aBuf, aBufLength); + static_cast(Server::sServer)->ReceiveTask(aBuf, aBufLength); } Console::Console(Instance *aInstance) - : mCallback(NULL) + : Server(aInstance) + , mCallback(NULL) , mContext(NULL) - , mInterpreter(aInstance) { } diff --git a/src/cli/cli_console.hpp b/src/cli/cli_console.hpp index c1cb15a5f..7f1078df1 100644 --- a/src/cli/cli_console.hpp +++ b/src/cli/cli_console.hpp @@ -107,8 +107,6 @@ private: otCliConsoleOutputCallback mCallback; void * mContext; - - Interpreter mInterpreter; }; } // namespace Cli diff --git a/src/cli/cli_dataset.cpp b/src/cli/cli_dataset.cpp index 91699a50d..5cd9a266d 100644 --- a/src/cli/cli_dataset.cpp +++ b/src/cli/cli_dataset.cpp @@ -40,6 +40,7 @@ #include #include "cli/cli.hpp" +#include "cli/cli_server.hpp" namespace ot { namespace Cli { diff --git a/src/cli/cli_dataset.hpp b/src/cli/cli_dataset.hpp index fea828b6c..f1b8d0e01 100644 --- a/src/cli/cli_dataset.hpp +++ b/src/cli/cli_dataset.hpp @@ -40,8 +40,6 @@ #include -#include "cli/cli_server.hpp" - namespace ot { namespace Cli { diff --git a/src/cli/cli_instance.cpp b/src/cli/cli_instance.cpp index c3f19ecb0..d20a5863d 100644 --- a/src/cli/cli_instance.cpp +++ b/src/cli/cli_instance.cpp @@ -37,6 +37,7 @@ #include #include "cli.hpp" +#include "cli_server.hpp" #include "common/debug.hpp" #include "utils/wrap_string.h" diff --git a/src/cli/cli_server.cpp b/src/cli/cli_server.cpp new file mode 100644 index 000000000..4dfeb0d38 --- /dev/null +++ b/src/cli/cli_server.cpp @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2019, The OpenThread Authors. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the copyright holder nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +/** + * @file + * This file provides the static fields for CLI server. + */ + +#include "cli_server.hpp" + +namespace ot { +namespace Cli { + +Server *Server::sServer; + +} // namespace Cli +} // namespace ot diff --git a/src/cli/cli_server.hpp b/src/cli/cli_server.hpp index 3e9ff40d3..1f5f69c65 100644 --- a/src/cli/cli_server.hpp +++ b/src/cli/cli_server.hpp @@ -36,6 +36,8 @@ #include "openthread-core-config.h" +#include "cli/cli.hpp" + namespace ot { namespace Cli { @@ -46,6 +48,12 @@ namespace Cli { class Server { public: + Server(Instance *aInstance) + : mInterpreter(aInstance) + { + ; + } + /** * This method delivers raw characters to the client. * @@ -80,6 +88,19 @@ public: OT_UNUSED_VARIABLE(aFormat); return -1; } + + /** + * This method returns a reference to the interpreter object. + * + * @returns A reference to the interpreter object. + * + */ + Interpreter &GetInterpreter(void) { return mInterpreter; } + + static Server *sServer; + +protected: + Interpreter mInterpreter; }; } // namespace Cli diff --git a/src/cli/cli_uart.cpp b/src/cli/cli_uart.cpp index 824bfbcbf..c6496f2dd 100644 --- a/src/cli/cli_uart.cpp +++ b/src/cli/cli_uart.cpp @@ -87,47 +87,45 @@ namespace ot { namespace Cli { -Uart *Uart::sUartServer; - static otDEFINE_ALIGNED_VAR(sCliUartRaw, sizeof(Uart), uint64_t); extern "C" void otCliUartInit(otInstance *aInstance) { Instance *instance = static_cast(aInstance); - Uart::sUartServer = new (&sCliUartRaw) Uart(instance); + Server::sServer = new (&sCliUartRaw) Uart(instance); } extern "C" void otCliUartSetUserCommands(const otCliCommand *aUserCommands, uint8_t aLength) { - Uart::sUartServer->GetInterpreter().SetUserCommands(aUserCommands, aLength); + Server::sServer->GetInterpreter().SetUserCommands(aUserCommands, aLength); } extern "C" void otCliUartOutputBytes(const uint8_t *aBytes, uint8_t aLength) { - Uart::sUartServer->GetInterpreter().OutputBytes(aBytes, aLength); + Server::sServer->GetInterpreter().OutputBytes(aBytes, aLength); } extern "C" void otCliUartOutputFormat(const char *aFmt, ...) { va_list aAp; va_start(aAp, aFmt); - Uart::sUartServer->OutputFormatV(aFmt, aAp); + static_cast(Server::sServer)->OutputFormatV(aFmt, aAp); va_end(aAp); } extern "C" void otCliUartOutput(const char *aString, uint16_t aLength) { - Uart::sUartServer->Output(aString, aLength); + Server::sServer->Output(aString, aLength); } extern "C" void otCliUartAppendResult(otError aError) { - Uart::sUartServer->GetInterpreter().AppendResult(aError); + Server::sServer->GetInterpreter().AppendResult(aError); } Uart::Uart(Instance *aInstance) - : mInterpreter(aInstance) + : Server(aInstance) { mRxLength = 0; mTxHead = 0; @@ -139,7 +137,7 @@ Uart::Uart(Instance *aInstance) extern "C" void otPlatUartReceived(const uint8_t *aBuf, uint16_t aBufLength) { - Uart::sUartServer->ReceiveTask(aBuf, aBufLength); + static_cast(Server::sServer)->ReceiveTask(aBuf, aBufLength); } void Uart::ReceiveTask(const uint8_t *aBuf, uint16_t aBufLength) @@ -339,7 +337,7 @@ exit: extern "C" void otPlatUartSendDone(void) { - Uart::sUartServer->SendDoneTask(); + static_cast(Server::sServer)->SendDoneTask(); } void Uart::SendDoneTask(void) @@ -356,10 +354,10 @@ extern "C" void otCliPlatLogv(otLogLevel aLogLevel, otLogRegion aLogRegion, cons OT_UNUSED_VARIABLE(aLogLevel); OT_UNUSED_VARIABLE(aLogRegion); - VerifyOrExit(Uart::sUartServer != NULL); + VerifyOrExit(Server::sServer != NULL); - Uart::sUartServer->OutputFormatV(aFormat, aArgs); - Uart::sUartServer->OutputFormat("\r\n"); + static_cast(Server::sServer)->OutputFormatV(aFormat, aArgs); + Server::sServer->OutputFormat("\r\n"); exit: return; diff --git a/src/cli/cli_uart.hpp b/src/cli/cli_uart.hpp index f3e5133ff..c1012efd0 100644 --- a/src/cli/cli_uart.hpp +++ b/src/cli/cli_uart.hpp @@ -92,19 +92,9 @@ public: */ int OutputFormatV(const char *aFmt, va_list aAp); - /** - * This method returns a reference to the interpreter object. - * - * @returns A reference to the interpreter object. - * - */ - Interpreter &GetInterpreter(void) { return mInterpreter; } - void ReceiveTask(const uint8_t *aBuf, uint16_t aBufLength); void SendDoneTask(void); - static Uart *sUartServer; - private: enum { @@ -125,8 +115,6 @@ private: uint16_t mSendLength; - Interpreter mInterpreter; - friend class Interpreter; }; diff --git a/src/cli/cli_udp.cpp b/src/cli/cli_udp.cpp index b04319af8..fd37e2616 100644 --- a/src/cli/cli_udp.cpp +++ b/src/cli/cli_udp.cpp @@ -37,6 +37,7 @@ #include #include "cli/cli.hpp" +#include "cli/cli_server.hpp" #include "common/encoding.hpp" using ot::Encoding::BigEndian::HostSwap16;