From e296446449f2df6f5bfe3d10f83889b63d81eaa7 Mon Sep 17 00:00:00 2001 From: cuiziwei Date: Tue, 28 Apr 2026 17:00:56 +0800 Subject: [PATCH 1/3] system/popen: add dpopen/dpclose fd-based interface Add dpopen()/dpclose() as the descriptor-based counterpart of popen()/pclose(), analogous to how dprintf() relates to fprintf(). dpopen() returns a raw file descriptor instead of a FILE stream, avoiding the stdio.h dependency for callers that only need an fd. Refactor popen() as a thin wrapper: dpopen() + fdopen() + FILE container. All pipe creation and process spawning logic now lives in dpopen.c. Also remove the hard dependency on NSH_LIBRARY from SYSTEM_POPEN. When NSH is available, commands are executed through sh -c with full shell syntax support. When NSH is not available, commands are split by whitespace and executed directly via posix_spawnp(). Add CONFIG_SYSTEM_POPEN_MAXARGUMENTS (default 7) to control the argv array size for the no-shell path. Signed-off-by: cuiziwei --- system/popen/CMakeLists.txt | 2 +- system/popen/Kconfig | 19 ++- system/popen/Makefile | 4 +- system/popen/dpopen.c | 321 ++++++++++++++++++++++++++++++++++++ system/popen/popen.c | 298 +++++---------------------------- 5 files changed, 379 insertions(+), 265 deletions(-) create mode 100644 system/popen/dpopen.c diff --git a/system/popen/CMakeLists.txt b/system/popen/CMakeLists.txt index 78add77e62a..353205d242c 100644 --- a/system/popen/CMakeLists.txt +++ b/system/popen/CMakeLists.txt @@ -21,5 +21,5 @@ # ############################################################################## if(CONFIG_SYSTEM_POPEN) - target_sources(apps PRIVATE popen.c) + target_sources(apps PRIVATE dpopen.c popen.c) endif() diff --git a/system/popen/Kconfig b/system/popen/Kconfig index 94041b0e05e..606efa032f2 100644 --- a/system/popen/Kconfig +++ b/system/popen/Kconfig @@ -4,14 +4,25 @@ # config SYSTEM_POPEN - bool "popen()/pclose() Functions" + bool "popen()/pclose()/dpopen()/dpclose() Functions" default n select SCHED_WAITPID depends on NSH_LIBRARY && PIPES ---help--- - Enable support for the popen() and pclose() interfaces. - This will support execution of NSH commands from C code with - pipe communications with the shell. + Enable support for the popen(), pclose(), dpopen(), and + dpclose() interfaces. + + dpopen()/dpclose() are the descriptor-based counterparts + of popen()/pclose(), analogous to how dprintf() relates + to fprintf(). They return a raw file descriptor instead + of a FILE stream, avoiding the stdio.h dependency. + + popen()/pclose() are thin wrappers around dpopen()/dpclose() + that additionally wrap the fd in a FILE stream. + + Commands are executed through the NSH shell (sh -c command), + supporting full shell syntax including pipes, redirects, + and globbing. if SYSTEM_POPEN diff --git a/system/popen/Makefile b/system/popen/Makefile index fbd9582745d..7e8887de678 100644 --- a/system/popen/Makefile +++ b/system/popen/Makefile @@ -22,8 +22,8 @@ include $(APPDIR)/Make.defs -# popen()/pclose functions +# dpopen()/dpclose() core and popen()/pclose() FILE wrappers -CSRCS = popen.c +CSRCS = dpopen.c popen.c include $(APPDIR)/Application.mk diff --git a/system/popen/dpopen.c b/system/popen/dpopen.c new file mode 100644 index 00000000000..dd8bc9b7dc2 --- /dev/null +++ b/system/popen/dpopen.c @@ -0,0 +1,321 @@ +/**************************************************************************** + * apps/system/popen/dpopen.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#if defined(CONFIG_NET_LOCAL) && defined(CONFIG_NET_LOCAL_STREAM) +# include +#endif + +#include "nshlib/nshlib.h" + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: dpopen + * + * Description: + * Execute a command and return a raw file descriptor connected to the + * child process via a pipe, along with the child PID. + * + * This is the descriptor-based counterpart of popen(), analogous to how + * dprintf() relates to fprintf(). It avoids the FILE stream layer, + * making it suitable for callers that only need a raw fd and want to + * avoid the stdio.h dependency. + * + * When NSH is available, commands are executed through the shell + * (sh -c command), supporting full shell syntax. + * + * Input Parameters: + * command - The command string to execute + * oflag - O_RDONLY to read child stdout, O_WRONLY to write child stdin, + * O_RDWR for bidirectional socket mode + * pid - Location to return the child process ID + * + * Returned Value: + * A valid file descriptor on success, or -1 on failure with errno set. + * + ****************************************************************************/ + +int dpopen(FAR const char *command, int oflag, FAR pid_t *pid) +{ + struct sched_param param; + posix_spawnattr_t attr; + posix_spawn_file_actions_t file_actions; + FAR char *argv[4]; + int fd[2]; + int childfd; + int parentfd; + int errcode; + + if (command == NULL || pid == NULL) + { + errno = EINVAL; + return -1; + } + + /* Create a pipe or socketpair. fd[0] refers to the read end of the + * pipe; fd[1] refers to the write end of the pipe. + */ + + if ((oflag & O_RDWR) == O_RDWR) + { +#if defined(CONFIG_NET_LOCAL) && defined(CONFIG_NET_LOCAL_STREAM) + /* Create a socketpair for bidirectional communication */ + + if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, fd) < 0) + { + return -1; + } + + childfd = fd[0]; + parentfd = fd[1]; +#else + errno = ENOTSUP; + return -1; +#endif + } + else if (pipe2(fd, O_CLOEXEC) < 0) + { + return -1; + } + else if ((oflag & O_ACCMODE) == O_RDONLY) + { + /* Pipe is the output from the child */ + + childfd = fd[1]; /* Child writes to pipe */ + parentfd = fd[0]; /* Parent reads from pipe */ + } + else if ((oflag & O_ACCMODE) == O_WRONLY) + { + /* Pipe is the input to the child */ + + childfd = fd[0]; /* Child reads from pipe */ + parentfd = fd[1]; /* Parent writes to pipe */ + } + else + { + errcode = EINVAL; + goto errout_with_pipe; + } + + /* Initialize attributes for task_spawn() (or posix_spawn()). */ + + errcode = posix_spawnattr_init(&attr); + if (errcode != 0) + { + goto errout_with_pipe; + } + + errcode = posix_spawn_file_actions_init(&file_actions); + if (errcode != 0) + { + goto errout_with_attr; + } + + /* Set the correct stack size and priority */ + + param.sched_priority = CONFIG_SYSTEM_POPEN_PRIORITY; + errcode = posix_spawnattr_setschedparam(&attr, ¶m); + if (errcode != 0) + { + goto errout_with_actions; + } + +#ifndef CONFIG_SYSTEM_POPEN_SHPATH + errcode = posix_spawnattr_setstacksize(&attr, + CONFIG_SYSTEM_POPEN_STACKSIZE); + if (errcode != 0) + { + goto errout_with_actions; + } +#endif + + /* If robin robin scheduling is enabled, then set the scheduling policy + * of the new task to SCHED_RR before it has a chance to run. + */ + +#if CONFIG_RR_INTERVAL > 0 + errcode = posix_spawnattr_setschedpolicy(&attr, SCHED_RR); + if (errcode != 0) + { + goto errout_with_actions; + } + + errcode = posix_spawnattr_setflags(&attr, + POSIX_SPAWN_SETSCHEDPARAM | + POSIX_SPAWN_SETSCHEDULER); + if (errcode != 0) + { + goto errout_with_actions; + } + +#else + errcode = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSCHEDPARAM); + if (errcode != 0) + { + goto errout_with_actions; + } + +#endif + + /* Redirect child's stdin or stdout to the pipe */ + + if ((oflag & O_RDWR) == O_RDWR) + { + errcode = posix_spawn_file_actions_adddup2(&file_actions, childfd, + STDIN_FILENO); + if (errcode != 0) + { + goto errout_with_actions; + } + + errcode = posix_spawn_file_actions_adddup2(&file_actions, childfd, + STDOUT_FILENO); + if (errcode != 0) + { + goto errout_with_actions; + } + } + else + { + errcode = posix_spawn_file_actions_adddup2(&file_actions, childfd, + (oflag & O_ACCMODE) == O_RDONLY ? + STDOUT_FILENO : STDIN_FILENO); + if (errcode != 0) + { + goto errout_with_actions; + } + } + + /* Call task_spawn() (or posix_spawn), re-directing stdin or stdout + * appropriately. + */ + + argv[1] = "-c"; + argv[2] = (FAR char *)command; + argv[3] = NULL; + +#ifdef CONFIG_SYSTEM_POPEN_SHPATH + argv[0] = CONFIG_SYSTEM_POPEN_SHPATH; + errcode = posix_spawn(pid, argv[0], &file_actions, + &attr, argv, NULL); +#else + *pid = task_spawn("dpopen", nsh_system, &file_actions, + &attr, argv + 1, NULL); + if (*pid < 0) + { + errcode = -*pid; + } +#endif + + if (errcode != 0) + { + serr("ERROR: dpopen spawn failed: %d\n", errcode); + goto errout_with_pipe; + } + + /* Free attributes and file actions */ + + posix_spawn_file_actions_destroy(&file_actions); + posix_spawnattr_destroy(&attr); + + /* Close the child's end in the parent */ + + if (!(oflag & O_CLOEXEC)) + { + ioctl(parentfd, FIONCLEX, 0); + } + + close(childfd); + + return parentfd; + +errout_with_actions: + posix_spawn_file_actions_destroy(&file_actions); + +errout_with_attr: + posix_spawnattr_destroy(&attr); + +errout_with_pipe: + close(fd[0]); + close(fd[1]); + errno = errcode; + return -1; +} + +/**************************************************************************** + * Name: dpclose + * + * Description: + * Close a file descriptor opened by dpopen() and wait for the child + * process to terminate. + * + * This is the descriptor-based counterpart of pclose(). + * + * Input Parameters: + * fd - The file descriptor returned by dpopen() + * pid - The child process ID returned by dpopen() + * + * Returned Value: + * The child termination status on success, or -1 on failure with + * errno set. + * + ****************************************************************************/ + +int dpclose(int fd, pid_t pid) +{ +#ifdef CONFIG_SCHED_WAITPID + int status; + int ret; +#endif + + if (fd >= 0) + { + close(fd); + } + +#ifdef CONFIG_SCHED_WAITPID + ret = waitpid(pid, &status, 0); + if (ret < 0) + { + return ERROR; + } + + return status; +#else + return 0; +#endif +} diff --git a/system/popen/popen.c b/system/popen/popen.c index b52fbaafe85..4f46e6cd2b9 100644 --- a/system/popen/popen.c +++ b/system/popen/popen.c @@ -26,21 +26,13 @@ #include -#include -#include #include #include #include #include -#include -#include -#include -#include #include #include -#include "nshlib/nshlib.h" - /**************************************************************************** * Private Types ****************************************************************************/ @@ -69,17 +61,9 @@ struct popen_file_s * executed command, and will return a pointer to a stream that can be * used to either read from or write to the pipe. * - * The environment of the executed command will be as if a child process - * were created within the popen() call using the fork() function, and the - * child invoked the sh utility using the call: - * - * execl(shell path, "sh", "-c", command, NULL); - * - * where shell path is an unspecified pathname for the sh utility. - * - * The popen() function will ensure that any streams from previous popen() - * calls that remain open in the parent process are closed in the new child - * process. + * This is a thin wrapper around dpopen() that wraps the returned file + * descriptor in a FILE stream, analogous to how fprintf() relates to + * dprintf(). * * The mode argument to popen() is a string that specifies I/O mode: * @@ -95,18 +79,12 @@ struct popen_file_s * the stream pointer returned by popen(), will be the writable end of * the pipe. * - * If mode is any other value, the result is undefined. - * - * After popen(), both the parent and the child process will be capable of - * executing independently before either terminates. - * - * Pipe streams are byte-oriented. - * * Input Parameters: - * command + * command - The command string to execute + * mode - "r" or "w" * * Returned Value: - * A non-NULLFILE stream connected to the shell instance is returned on + * A non-NULL FILE stream connected to the child process is returned on * success. NULL is returned on any failure with the errno variable set * appropriately. * @@ -115,244 +93,67 @@ struct popen_file_s FILE *popen(FAR const char *command, FAR const char *mode) { FAR struct popen_file_s *container; - struct sched_param param; - posix_spawnattr_t attr; - posix_spawn_file_actions_t file_actions; - FAR char *argv[4]; - int fd[2]; - int oldfd[2]; - int newfd[2]; - int retfd; - int errcode; - int result = 0; - bool rw = false; + int oflag; + int fd; /* Allocate a container for returned FILE stream */ container = (FAR struct popen_file_s *)malloc(sizeof(struct popen_file_s)); if (container == NULL) { - errcode = ENOMEM; - goto errout; + errno = ENOMEM; + return NULL; } - oldfd[1] = 0; - newfd[1] = 0; - - /* Create a pipe. fd[0] refers to the read end of the pipe; fd[1] refers - * to the write end of the pipe. - * Is the pipe the input to the shell? Or the output? - */ - - if (strcmp(mode, "r") == 0 && - (result = pipe2(fd, O_CLOEXEC)) >= 0) - { - /* Pipe is the output from the shell */ + /* Map mode string to open flags */ - oldfd[0] = 1; /* Replace stdout with the write side of the pipe */ - newfd[0] = fd[1]; - retfd = fd[0]; /* Use read side of the pipe to create the return stream */ - } - else if (strcmp(mode, "w") == 0 && - (result = pipe2(fd, O_CLOEXEC)) >= 0) + if (strstr(mode, "r+") || strstr(mode, "w+")) { - /* Pipe is the input to the shell */ - - oldfd[0] = 0; /* Replace stdin with the read side of the pipe */ - newfd[0] = fd[0]; - retfd = fd[1]; /* Use write side of the pipe to create the return stream */ + oflag = O_RDWR; } - - /* Create a socketpair. Using fd[0] as the input and output to the shell */ - -#if defined(CONFIG_NET_LOCAL) && defined(CONFIG_NET_LOCAL_STREAM) - else if ((strcmp(mode, "r+") == 0 || strcmp(mode, "w+") == 0) && - (result = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, - 0, fd)) >= 0) + else if (strstr(mode, "r")) { - /* Socketpair is the input/output to the shell */ - - rw = true; - oldfd[0] = 0; /* Replace stdin with the one side of a socket pair */ - newfd[0] = fd[0]; - oldfd[1] = 1; /* Replace stdout with the one side of a socket pair */ - newfd[1] = fd[0]; - retfd = fd[1]; /* Use other side of the socket pair to create the return stream */ + oflag = O_RDONLY; } -#endif - else if (result < 0) + else if (strstr(mode, "w")) { - errcode = errno; - goto errout_with_container; + oflag = O_WRONLY; } else { - errcode = EINVAL; - goto errout_with_pipe; - } - - /* Create the FILE stream return reference */ - - container->original = fdopen(retfd, mode); - if (container->original == NULL) - { - errcode = errno; - goto errout_with_pipe; - } - - /* Initialize attributes for task_spawn() (or posix_spawn()). */ - - errcode = posix_spawnattr_init(&attr); - if (errcode != 0) - { - goto errout_with_stream; - } - - errcode = posix_spawn_file_actions_init(&file_actions); - if (errcode != 0) - { - goto errout_with_attrs; - } - - /* Set the correct stack size and priority */ - - param.sched_priority = CONFIG_SYSTEM_POPEN_PRIORITY; - errcode = posix_spawnattr_setschedparam(&attr, ¶m); - if (errcode != 0) - { - goto errout_with_actions; - } - -#ifndef CONFIG_SYSTEM_POPEN_SHPATH - errcode = posix_spawnattr_setstacksize(&attr, - CONFIG_SYSTEM_POPEN_STACKSIZE); - if (errcode != 0) - { - goto errout_with_actions; - } -#endif - - /* If robin robin scheduling is enabled, then set the scheduling policy - * of the new task to SCHED_RR before it has a chance to run. - */ - -#if CONFIG_RR_INTERVAL > 0 - errcode = posix_spawnattr_setschedpolicy(&attr, SCHED_RR); - if (errcode != 0) - { - goto errout_with_actions; + free(container); + errno = EINVAL; + return NULL; } - errcode = posix_spawnattr_setflags(&attr, - POSIX_SPAWN_SETSCHEDPARAM | - POSIX_SPAWN_SETSCHEDULER); - if (errcode != 0) + if (strchr(mode, 'e') != NULL) { - goto errout_with_actions; + oflag |= O_CLOEXEC; } -#else - errcode = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSCHEDPARAM); - if (errcode != 0) - { - goto errout_with_actions; - } - -#endif - - /* Redirect input or output as determined by the mode parameter */ - - errcode = posix_spawn_file_actions_adddup2(&file_actions, - newfd[0], oldfd[0]); - if (errcode != 0) - { - goto errout_with_actions; - } - - if (rw) - { - errcode = posix_spawn_file_actions_adddup2(&file_actions, - newfd[1], oldfd[1]); - if (errcode != 0) - { - goto errout_with_actions; - } - } - - /* Call task_spawn() (or posix_spawn), re-directing stdin or stdout - * appropriately. - */ - - argv[1] = "-c"; - argv[2] = (FAR char *)command; - argv[3] = NULL; - -#ifdef CONFIG_SYSTEM_POPEN_SHPATH - argv[0] = CONFIG_SYSTEM_POPEN_SHPATH; - errcode = posix_spawn(&container->shell, argv[0], &file_actions, - &attr, argv, NULL); -#else - container->shell = task_spawn("popen", nsh_system, &file_actions, - &attr, argv + 1, NULL); - if (container->shell < 0) - { - errcode = -container->shell; - } -#endif - - if (errcode != 0) - { - serr("ERROR: Spawn failed: %d\n", errcode); - goto errout_with_actions; - } - - /* We can close the 'newfd' now. It is no longer useful on this side of - * the interface. - */ - - close(newfd[0]); + /* Use dpopen() to do the real work */ - if (rw && newfd[0] != newfd[1]) + fd = dpopen(command, oflag, &container->shell); + if (fd < 0) { - close(newfd[1]); + free(container); + return NULL; } - /* Free attributes and file actions. Ignoring return values in the case - * of an error. - */ + /* Wrap the raw fd in a FILE stream */ - posix_spawn_file_actions_destroy(&file_actions); - posix_spawnattr_destroy(&attr); - - if (strchr(mode, 'e') == NULL) + container->original = fdopen(fd, mode); + if (container->original == NULL) { - ioctl(retfd, FIOCLEX, 0); + int errcode = errno; + dpclose(fd, container->shell); + free(container); + errno = errcode; + return NULL; } - /* Finale and return input input/output stream */ - memcpy(&container->copy, container->original, sizeof(FILE)); return &container->copy; - -errout_with_actions: - posix_spawn_file_actions_destroy(&file_actions); - -errout_with_attrs: - posix_spawnattr_destroy(&attr); - -errout_with_stream: - fclose(container->original); - -errout_with_pipe: - close(fd[0]); - close(fd[1]); - -errout_with_container: - free(container); - -errout: - errno = errcode; - return NULL; } /**************************************************************************** @@ -388,12 +189,12 @@ FILE *popen(FAR const char *command, FAR const char *mode) * If the argument stream to pclose() is not a pointer to a stream created * by popen(), the result of pclose() is undefined. * - * Description: + * Input Parameters: * stream - The stream reference returned by a previous call to popen() * * Returned Value: - * Zero (OK) is returned on success; otherwise -1 (ERROR) is returned and - * the errno variable is set appropriately. + * The child termination status on success, or -1 (ERROR) on failure + * with errno set. * ****************************************************************************/ @@ -402,12 +203,7 @@ int pclose(FILE *stream) FAR struct popen_file_s *container = (FAR struct popen_file_s *)stream; FILE *original; pid_t shell; -#ifdef CONFIG_SCHED_WAITPID - int status; - int result; -#endif - DEBUGASSERT(container != NULL && container->original != NULL); original = container->original; /* Set the state of the original file descriptor to the state of the @@ -417,7 +213,7 @@ int pclose(FILE *stream) memcpy(original, &container->copy, sizeof(FILE)); /* Then close the original and free the container (saving the PID of the - * shell process) + * shell process). Pass -1 to dpclose since fclose already closed the fd. */ fclose(original); @@ -425,19 +221,5 @@ int pclose(FILE *stream) shell = container->shell; free(container); -#ifdef CONFIG_SCHED_WAITPID - /* Wait for the shell to exit, retrieving the return value if available. */ - - result = waitpid(shell, &status, 0); - if (result < 0) - { - /* The errno has already been set */ - - return ERROR; - } - - return status; -#else - return OK; -#endif + return dpclose(-1, shell); } From 5c654d5b8ed40df00f61e56ef3fd6fef7c90f6fe Mon Sep 17 00:00:00 2001 From: cuiziwei Date: Tue, 28 Apr 2026 17:32:22 +0800 Subject: [PATCH 2/3] system/popen: support no-shell mode via posix_spawnp When NSH_LIBRARY is not available, dpopen()/popen() can still execute commands by splitting the command string by whitespace and calling posix_spawnp() directly. Shell syntax (pipes, redirects, globbing) is not supported in this mode. Add CONFIG_SYSTEM_POPEN_MAXARGUMENTS (default 7) to control the argv array size for the no-shell path. Remove the hard dependency on NSH_LIBRARY from SYSTEM_POPEN so the feature can be used in minimal configurations without a shell. Signed-off-by: cuiziwei --- system/popen/Kconfig | 27 ++++++++++++++++---- system/popen/dpopen.c | 59 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/system/popen/Kconfig b/system/popen/Kconfig index 606efa032f2..6256af7c4ea 100644 --- a/system/popen/Kconfig +++ b/system/popen/Kconfig @@ -7,7 +7,7 @@ config SYSTEM_POPEN bool "popen()/pclose()/dpopen()/dpclose() Functions" default n select SCHED_WAITPID - depends on NSH_LIBRARY && PIPES + depends on PIPES ---help--- Enable support for the popen(), pclose(), dpopen(), and dpclose() interfaces. @@ -20,16 +20,20 @@ config SYSTEM_POPEN popen()/pclose() are thin wrappers around dpopen()/dpclose() that additionally wrap the fd in a FILE stream. - Commands are executed through the NSH shell (sh -c command), - supporting full shell syntax including pipes, redirects, - and globbing. + When NSH is available (NSH_LIBRARY), commands are executed + through the shell (sh -c command), supporting full shell + syntax including pipes, redirects, and globbing. + + When NSH is not available, commands are executed directly + via posix_spawnp(). The command string must be a simple + "program arg1 arg2" form without shell syntax. if SYSTEM_POPEN config SYSTEM_POPEN_SHPATH string "Path to shell command" default "/bin/sh" - depends on SYSTEM_NSH=m || BUILD_KERNEL + depends on (SYSTEM_NSH=m || BUILD_KERNEL) && NSH_LIBRARY ---help--- This is the full path to the program in a mounted file system that implements the system() command. That is, a program that starts the @@ -53,4 +57,17 @@ config SYSTEM_POPEN_PRIORITY ---help--- The priority of the shell. +config SYSTEM_POPEN_MAXARGUMENTS + int "Maximum number of command arguments (no-shell mode)" + default 7 + depends on !NSH_LIBRARY + ---help--- + When NSH is not available, dpopen()/popen() splits the command + string by whitespace into an argv array. This sets the maximum + number of arguments (excluding the program name and the + terminating NULL). + + This is analogous to CONFIG_NSH_MAXARGUMENTS but applies + only to the no-shell path. + endif diff --git a/system/popen/dpopen.c b/system/popen/dpopen.c index dd8bc9b7dc2..785fcc20244 100644 --- a/system/popen/dpopen.c +++ b/system/popen/dpopen.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -37,7 +38,17 @@ # include #endif -#include "nshlib/nshlib.h" +#ifdef CONFIG_NSH_LIBRARY +# include "nshlib/nshlib.h" +#endif + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#ifndef CONFIG_NSH_LIBRARY +# define DPOPEN_MAX_ARGV (CONFIG_SYSTEM_POPEN_MAXARGUMENTS + 1) +#endif /**************************************************************************** * Public Functions @@ -58,6 +69,10 @@ * When NSH is available, commands are executed through the shell * (sh -c command), supporting full shell syntax. * + * When NSH is not available, the command is split by whitespace and + * executed directly via posix_spawnp(). Shell syntax (pipes, redirects, + * globbing) is not supported in this mode. + * * Input Parameters: * command - The command string to execute * oflag - O_RDONLY to read child stdout, O_WRONLY to write child stdin, @@ -74,7 +89,14 @@ int dpopen(FAR const char *command, int oflag, FAR pid_t *pid) struct sched_param param; posix_spawnattr_t attr; posix_spawn_file_actions_t file_actions; +#ifdef CONFIG_NSH_LIBRARY FAR char *argv[4]; +#else + char cmdbuf[PATH_MAX]; + FAR char *argv[DPOPEN_MAX_ARGV]; + FAR char *saveptr; + int argc = 0; +#endif int fd[2]; int childfd; int parentfd; @@ -224,21 +246,52 @@ int dpopen(FAR const char *command, int oflag, FAR pid_t *pid) * appropriately. */ +#ifdef CONFIG_NSH_LIBRARY + /* Shell mode: execute command through sh -c */ + argv[1] = "-c"; argv[2] = (FAR char *)command; argv[3] = NULL; -#ifdef CONFIG_SYSTEM_POPEN_SHPATH +# ifdef CONFIG_SYSTEM_POPEN_SHPATH argv[0] = CONFIG_SYSTEM_POPEN_SHPATH; errcode = posix_spawn(pid, argv[0], &file_actions, &attr, argv, NULL); -#else +# else *pid = task_spawn("dpopen", nsh_system, &file_actions, &attr, argv + 1, NULL); if (*pid < 0) { errcode = -*pid; } +# endif +#else + /* No-shell mode: split command and execute directly via posix_spawnp. + * The command must be in "program arg1 arg2" form -- no shell syntax. + */ + + if (strlcpy(cmdbuf, command, sizeof(cmdbuf)) >= sizeof(cmdbuf)) + { + errcode = ENAMETOOLONG; + goto errout_with_actions; + } + + do + { + argv[argc] = strtok_r(argc ? NULL : cmdbuf, " \t", &saveptr); + } + while (argv[argc] != NULL && ++argc < DPOPEN_MAX_ARGV - 1); + + argv[argc] = NULL; + + if (argc == 0) + { + errcode = EINVAL; + goto errout_with_actions; + } + + errcode = posix_spawnp(pid, argv[0], &file_actions, + &attr, argv, NULL); #endif if (errcode != 0) From 00a219843c0c215a129d1629a159e2f4deb0b90a Mon Sep 17 00:00:00 2001 From: cuiziwei Date: Tue, 28 Apr 2026 17:29:12 +0800 Subject: [PATCH 3/3] testing/libc/popen: add popen/dpopen test for shell and no-shell modes Add tests for popen()/pclose() and dpopen()/dpclose() that work in both shell (NSH) and no-shell (direct posix_spawnp) modes. The test binary doubles as its own target via the --echo sub-command. Tests: 1-4: popen - basic read, multi-arg, pclose, invalid mode 5-7: dpopen - basic read, multi-arg, dpclose Signed-off-by: cuiziwei --- testing/libc/popen/CMakeLists.txt | 33 ++++ testing/libc/popen/Kconfig | 16 ++ testing/libc/popen/Make.defs | 25 +++ testing/libc/popen/Makefile | 32 ++++ testing/libc/popen/popen_test.c | 250 ++++++++++++++++++++++++++++++ 5 files changed, 356 insertions(+) create mode 100644 testing/libc/popen/CMakeLists.txt create mode 100644 testing/libc/popen/Kconfig create mode 100644 testing/libc/popen/Make.defs create mode 100644 testing/libc/popen/Makefile create mode 100644 testing/libc/popen/popen_test.c diff --git a/testing/libc/popen/CMakeLists.txt b/testing/libc/popen/CMakeLists.txt new file mode 100644 index 00000000000..867a568910e --- /dev/null +++ b/testing/libc/popen/CMakeLists.txt @@ -0,0 +1,33 @@ +# ############################################################################## +# apps/testing/libc/popen/CMakeLists.txt +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed to the Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with this work for +# additional information regarding copyright ownership. The ASF licenses this +# file to you under the Apache License, Version 2.0 (the "License"); you may not +# use this file except in compliance with the License. You may obtain a copy of +# the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations under +# the License. +# +# ############################################################################## + +if(CONFIG_TESTING_POPEN) + nuttx_add_application( + NAME + popen_test + STACKSIZE + ${CONFIG_DEFAULT_TASK_STACKSIZE} + MODULE + ${CONFIG_TESTING_POPEN} + SRCS + popen_test.c) +endif() diff --git a/testing/libc/popen/Kconfig b/testing/libc/popen/Kconfig new file mode 100644 index 00000000000..73d4beb3661 --- /dev/null +++ b/testing/libc/popen/Kconfig @@ -0,0 +1,16 @@ +# +# For a description of the syntax of this configuration file, +# see the file kconfig-language.txt in the NuttX tools repository. +# + +config TESTING_POPEN + tristate "popen()/dpopen() test" + default n + depends on SYSTEM_POPEN + ---help--- + Test popen()/pclose() and dpopen()/dpclose() in both shell + and no-shell modes. The test binary doubles as its own + popen/dpopen target via --echo. + +if TESTING_POPEN +endif diff --git a/testing/libc/popen/Make.defs b/testing/libc/popen/Make.defs new file mode 100644 index 00000000000..eac91c500d5 --- /dev/null +++ b/testing/libc/popen/Make.defs @@ -0,0 +1,25 @@ +############################################################################ +# apps/testing/libc/popen/Make.defs +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. The +# ASF licenses this file to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance with the +# License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +############################################################################ + +ifneq ($(CONFIG_TESTING_POPEN),) +CONFIGURED_APPS += $(APPDIR)/testing/libc/popen +endif diff --git a/testing/libc/popen/Makefile b/testing/libc/popen/Makefile new file mode 100644 index 00000000000..1ef31c7ab42 --- /dev/null +++ b/testing/libc/popen/Makefile @@ -0,0 +1,32 @@ +############################################################################ +# apps/testing/libc/popen/Makefile +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. The +# ASF licenses this file to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance with the +# License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +############################################################################ + +include $(APPDIR)/Make.defs + +PROGNAME = popen_test +PRIORITY = SCHED_PRIORITY_DEFAULT +STACKSIZE = $(CONFIG_DEFAULT_TASK_STACKSIZE) +MODULE = $(CONFIG_TESTING_POPEN) + +MAINSRC = popen_test.c + +include $(APPDIR)/Application.mk diff --git a/testing/libc/popen/popen_test.c b/testing/libc/popen/popen_test.c new file mode 100644 index 00000000000..bbe3a7e59e7 --- /dev/null +++ b/testing/libc/popen/popen_test.c @@ -0,0 +1,250 @@ +/**************************************************************************** + * apps/testing/libc/popen/popen_test.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include +#include +#include +#include +#include + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static int test_popen_read(void) +{ + FILE *fp; + char buf[256]; + int found = 0; + + printf("[TEST 1] popen read...\n"); + fp = popen("popen_test --echo hello_popen", "r"); + if (fp == NULL) + { + printf("[FAIL] popen returned NULL\n"); + return 1; + } + + while (fgets(buf, sizeof(buf), fp) != NULL) + { + printf(" > %s", buf); + if (strstr(buf, "hello_popen")) + { + found = 1; + } + } + + pclose(fp); + printf("%s\n", found ? "[PASS]" : "[FAIL] missing output"); + return !found; +} + +static int test_popen_multiarg(void) +{ + FILE *fp; + char buf[256]; + int found = 0; + + printf("[TEST 2] popen multi-arg...\n"); + fp = popen("popen_test --echo aaa bbb ccc", "r"); + if (fp == NULL) + { + printf("[FAIL] popen returned NULL\n"); + return 1; + } + + while (fgets(buf, sizeof(buf), fp) != NULL) + { + printf(" > %s", buf); + if (strstr(buf, "aaa bbb ccc")) + { + found = 1; + } + } + + pclose(fp); + printf("%s\n", found ? "[PASS]" : "[FAIL] wrong output"); + return !found; +} + +static int test_pclose_complete(void) +{ + FILE *fp; + char buf[256]; + + printf("[TEST 3] pclose completes...\n"); + fp = popen("popen_test --echo ok", "r"); + if (fp == NULL) + { + printf("[FAIL] popen returned NULL\n"); + return 1; + } + + while (fgets(buf, sizeof(buf), fp) != NULL); + pclose(fp); + printf("[PASS]\n"); + return 0; +} + +static int test_popen_invalid_mode(void) +{ + FILE *fp; + + printf("[TEST 4] popen invalid mode...\n"); + fp = popen("popen_test", "x"); + if (fp == NULL) + { + printf("[PASS]\n"); + return 0; + } + + pclose(fp); + printf("[FAIL] should return NULL\n"); + return 1; +} + +static int test_dpopen_read(void) +{ + char buf[256]; + ssize_t n; + pid_t pid; + int found = 0; + int fd; + + printf("[TEST 5] dpopen read...\n"); + fd = dpopen("popen_test --echo hello_dpopen", O_RDONLY, &pid); + if (fd < 0) + { + printf("[FAIL] dpopen returned %d\n", fd); + return 1; + } + + while ((n = read(fd, buf, sizeof(buf) - 1)) > 0) + { + buf[n] = '\0'; + printf(" > %s", buf); + if (strstr(buf, "hello_dpopen")) + { + found = 1; + } + } + + dpclose(fd, pid); + printf("%s\n", found ? "[PASS]" : "[FAIL] missing output"); + return !found; +} + +static int test_dpopen_multiarg(void) +{ + char buf[256]; + ssize_t n; + pid_t pid; + int found = 0; + int fd; + + printf("[TEST 6] dpopen multi-arg...\n"); + fd = dpopen("popen_test --echo xxx yyy zzz", O_RDONLY, &pid); + if (fd < 0) + { + printf("[FAIL] dpopen returned %d\n", fd); + return 1; + } + + while ((n = read(fd, buf, sizeof(buf) - 1)) > 0) + { + buf[n] = '\0'; + printf(" > %s", buf); + if (strstr(buf, "xxx yyy zzz")) + { + found = 1; + } + } + + dpclose(fd, pid); + printf("%s\n", found ? "[PASS]" : "[FAIL] wrong output"); + return !found; +} + +static int test_dpclose_complete(void) +{ + char buf[256]; + pid_t pid; + int fd; + + printf("[TEST 7] dpclose completes...\n"); + fd = dpopen("popen_test --echo ok", O_RDONLY, &pid); + if (fd < 0) + { + printf("[FAIL] dpopen returned %d\n", fd); + return 1; + } + + while (read(fd, buf, sizeof(buf)) > 0); + dpclose(fd, pid); + printf("[PASS]\n"); + return 0; +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +int main(int argc, FAR char *argv[]) +{ + int fail = 0; + + /* Sub-command: echo args back, used as popen/dpopen target */ + + if (argc > 1 && strcmp(argv[1], "--echo") == 0) + { + int i; + for (i = 2; i < argc; i++) + { + printf("%s%s", i > 2 ? " " : "", argv[i]); + } + + printf("\n"); + return 0; + } + + printf("=== popen/dpopen test ===\n\n"); + + /* popen tests */ + + fail += test_popen_read(); + fail += test_popen_multiarg(); + fail += test_pclose_complete(); + fail += test_popen_invalid_mode(); + + /* dpopen tests */ + + fail += test_dpopen_read(); + fail += test_dpopen_multiarg(); + fail += test_dpclose_complete(); + + printf("\n=== Results: %d failed ===\n", fail); + return fail; +}