From 6fda0786edab8e821e1647cc4be08b4a7059cf4a Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Mon, 9 Nov 2020 15:31:11 +0800 Subject: [PATCH] [posix] forkpty-arg value as a single argument (#5772) --- src/posix/platform/hdlc_interface.cpp | 42 +++++++++---------- src/posix/platform/hdlc_interface.hpp | 2 +- src/posix/platform/system.cpp | 11 ++++- .../scripts/expect/posix-scan-tx-to-sleep.exp | 4 +- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/posix/platform/hdlc_interface.cpp b/src/posix/platform/hdlc_interface.cpp index 424a9af01..0ddc96733 100644 --- a/src/posix/platform/hdlc_interface.cpp +++ b/src/posix/platform/hdlc_interface.cpp @@ -60,10 +60,6 @@ #include "common/code_utils.hpp" #include "common/logging.hpp" -#ifndef SOCKET_UTILS_DEFAULT_SHELL -#define SOCKET_UTILS_DEFAULT_SHELL "/bin/sh" -#endif - #ifdef __APPLE__ #ifndef B230400 @@ -156,7 +152,7 @@ otError HdlcInterface::Init(const RadioUrl &aRadioUrl) #if OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE else if (S_ISREG(st.st_mode)) { - mSockFd = ForkPty(aRadioUrl.GetPath(), aRadioUrl.GetValue("forkpty-arg")); + mSockFd = ForkPty(aRadioUrl); VerifyOrExit(mSockFd != -1, error = OT_ERROR_INVALID_ARGS); } #endif // OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE @@ -586,7 +582,7 @@ exit: } #if OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE -int HdlcInterface::ForkPty(const char *aCommand, const char *aRadioUrl) +int HdlcInterface::ForkPty(const RadioUrl &aRadioUrl) { int fd = -1; int pid = -1; @@ -599,38 +595,40 @@ int HdlcInterface::ForkPty(const char *aCommand, const char *aRadioUrl) cfmakeraw(&tios); tios.c_cflag = CS8 | HUPCL | CREAD | CLOCAL; - VerifyOrExit((pid = forkpty(&fd, nullptr, &tios, nullptr)) != -1, perror("forkpty()")); + VerifyOrDie((pid = forkpty(&fd, nullptr, &tios, nullptr)) != -1, OT_EXIT_ERROR_ERRNO); } if (0 == pid) { - const int kMaxCommand = 255; - char cmd[kMaxCommand]; + constexpr int kMaxArguments = 32; + char * argv[kMaxArguments + 1]; + size_t index = 0; - if (aRadioUrl == nullptr) + argv[index++] = const_cast(aRadioUrl.GetPath()); + + for (const char *arg = nullptr; + index < OT_ARRAY_LENGTH(argv) && (arg = aRadioUrl.GetValue("forkpty-arg", arg)) != nullptr; + argv[index++] = const_cast(arg)) { - rval = snprintf(cmd, sizeof(cmd), "exec %s", aCommand); + } + + if (index < OT_ARRAY_LENGTH(argv)) + { + argv[index] = nullptr; } else { - rval = snprintf(cmd, sizeof(cmd), "exec %s %s", aCommand, aRadioUrl); + DieNowWithMessage("Too many arguments!", OT_EXIT_INVALID_ARGUMENTS); } - VerifyOrExit(rval > 0 && static_cast(rval) < sizeof(cmd), - fprintf(stderr, "NCP file and configuration is too long!"); - rval = -1); - VerifyOrExit((rval = execl(SOCKET_UTILS_DEFAULT_SHELL, SOCKET_UTILS_DEFAULT_SHELL, "-c", cmd, - static_cast(nullptr))) != -1, - perror("execl(OT_RCP)")); + VerifyOrDie((rval = execvp(argv[0], argv)) != -1, OT_EXIT_ERROR_ERRNO); } else { - VerifyOrExit((rval = fcntl(fd, F_GETFL)) != -1, perror("fcntl(F_GETFL)")); - VerifyOrExit((rval = fcntl(fd, F_SETFL, rval | O_NONBLOCK | O_CLOEXEC)) != -1, perror("fcntl(F_SETFL)")); + VerifyOrDie((rval = fcntl(fd, F_GETFL)) != -1, OT_EXIT_ERROR_ERRNO); + VerifyOrDie((rval = fcntl(fd, F_SETFL, rval | O_NONBLOCK | O_CLOEXEC)) != -1, OT_EXIT_ERROR_ERRNO); } -exit: - VerifyOrDie(rval == 0, OT_EXIT_ERROR_ERRNO); return fd; } #endif // OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE diff --git a/src/posix/platform/hdlc_interface.hpp b/src/posix/platform/hdlc_interface.hpp index ef93a669c..8249eeed7 100644 --- a/src/posix/platform/hdlc_interface.hpp +++ b/src/posix/platform/hdlc_interface.hpp @@ -208,7 +208,7 @@ private: int OpenFile(const RadioUrl &aRadioUrl); #if OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE - static int ForkPty(const char *aCommand, const char *aRadioUrl); + static int ForkPty(const RadioUrl &aRadioUrl); #endif enum diff --git a/src/posix/platform/system.cpp b/src/posix/platform/system.cpp index eda4231ae..74ad6e288 100644 --- a/src/posix/platform/system.cpp +++ b/src/posix/platform/system.cpp @@ -52,7 +52,16 @@ otInstance *otSysInit(otPlatformConfig *aPlatformConfig) ot::Posix::RadioUrl radioUrl(aPlatformConfig->mRadioUrl); #if OPENTHREAD_POSIX_VIRTUAL_TIME - virtualTimeInit(static_cast(atoi(radioUrl.GetValue("forkpty-arg")))); + // The last argument must be the node id + { + const char *nodeId = nullptr; + + for (const char *arg = nullptr; (arg = radioUrl.GetValue("forkpty-arg", arg)) != nullptr; nodeId = arg) + { + } + + virtualTimeInit(static_cast(atoi(nodeId))); + } #endif VerifyOrDie(radioUrl.GetPath() != nullptr, OT_EXIT_INVALID_ARGUMENTS); diff --git a/tests/scripts/expect/posix-scan-tx-to-sleep.exp b/tests/scripts/expect/posix-scan-tx-to-sleep.exp index 1330c844e..e10896a22 100755 --- a/tests/scripts/expect/posix-scan-tx-to-sleep.exp +++ b/tests/scripts/expect/posix-scan-tx-to-sleep.exp @@ -29,7 +29,7 @@ source "tests/scripts/expect/_common.exp" -spawn $env(OT_POSIX_APPS)/ot-cli "spinel+hdlc+uart://$env(OT_SIMULATION_APPS)/ncp/ot-rcp?forkpty-arg=--sleep-to-tx 1" +spawn $env(OT_POSIX_APPS)/ot-cli "spinel+hdlc+uart://$env(OT_SIMULATION_APPS)/ncp/ot-rcp?forkpty-arg=--sleep-to-tx&forkpty-arg=1" set node_1 $spawn_id expect_after { timeout { exit 1 } @@ -75,7 +75,7 @@ expect -re {(\d+)} set channel $expect_out(1,string) expect "Done" -spawn $env(OT_POSIX_APPS)/ot-cli "spinel+hdlc+uart://$env(OT_SIMULATION_APPS)/ncp/ot-rcp?forkpty-arg=--sleep-to-tx 2" +spawn $env(OT_POSIX_APPS)/ot-cli "spinel+hdlc+uart://$env(OT_SIMULATION_APPS)/ncp/ot-rcp?forkpty-arg=--sleep-to-tx&forkpty-arg=2" set node_2 $spawn_id expect_after { timeout { exit 1 }