[4.11 PATCH] libmisc/shell: Fix the handling of joel scripts in telnet
chrisj at rtems.org
chrisj at rtems.org
Tue Feb 18 03:07:47 UTC 2020
From: Chris Johns <chrisj at rtems.org>
- Fix the passing of std[in/out] to child threads
- Fix deleting of managed memory in the key destructor
- Only set the key in the main loop thread
- Only allocate a shell env outside of the main loop
- Fix memory leak if the task start fails
- Remove error level from shell env, it cannot be returned this way. Add
exit_code but the API is broken so it cannot be returned.
Closes #3877
---
cpukit/libmisc/shell/shell.c | 205 +++++++++++++++++++----------
cpukit/libmisc/shell/shell.h | 9 +-
testsuites/libtests/shell01/init.c | 123 ++++++++++++++++-
3 files changed, 265 insertions(+), 72 deletions(-)
diff --git a/cpukit/libmisc/shell/shell.c b/cpukit/libmisc/shell/shell.c
index 5f042f5855..fd2148acfd 100644
--- a/cpukit/libmisc/shell/shell.c
+++ b/cpukit/libmisc/shell/shell.c
@@ -40,20 +40,35 @@
#include <pthread.h>
#include <assert.h>
+#define SHELL_STD_DEBUG 0
+
+#if SHELL_STD_DEBUG
+static FILE* default_stdout;
+#define shell_std_debug(...) fprintf(default_stdout, __VA_ARGS__)
+#else
+#define shell_std_debug(...)
+#endif
+
const rtems_shell_env_t rtems_global_shell_env = {
.magic = rtems_build_name('S', 'E', 'N', 'V'),
+ .managed = false,
.devname = CONSOLE_DEVICE_NAME,
.taskname = "SHGL",
.exit_shell = false,
.forever = true,
- .errorlevel = -1,
.echo = false,
.cwd = "/",
.input = NULL,
.output = NULL,
.output_append = false,
+ .parent_stdin = NULL,
+ .parent_stdout = NULL,
+ .parent_stderr = NULL,
.wake_on_end = RTEMS_ID_NONE,
- .login_check = NULL
+ .exit_code = NULL,
+ .login_check = NULL,
+ .uid = 0,
+ .gid = 0
};
static pthread_once_t rtems_shell_once = PTHREAD_ONCE_INIT;
@@ -64,7 +79,7 @@ static pthread_key_t rtems_shell_current_env_key;
* Initialize the shell user/process environment information
*/
static rtems_shell_env_t *rtems_shell_init_env(
- rtems_shell_env_t *shell_env_p
+ rtems_shell_env_t *shell_env_parent
)
{
rtems_shell_env_t *shell_env;
@@ -72,11 +87,16 @@ static rtems_shell_env_t *rtems_shell_init_env(
shell_env = malloc(sizeof(rtems_shell_env_t));
if ( !shell_env )
return NULL;
- if ( !shell_env_p ) {
+
+ if ( shell_env_parent == NULL ) {
+ shell_env_parent = rtems_shell_get_current_env();
+ }
+ if ( shell_env_parent == NULL ) {
*shell_env = rtems_global_shell_env;
} else {
- *shell_env = *shell_env_p;
+ *shell_env = *shell_env_parent;
}
+ shell_env->managed = true;
shell_env->taskname = NULL;
return shell_env;
@@ -92,14 +112,16 @@ static void rtems_shell_env_free(
rtems_shell_env_t *shell_env;
shell_env = (rtems_shell_env_t *) ptr;
- if ( !ptr )
+ if ( ptr == NULL )
return;
- if ( shell_env->input )
- free((void *)shell_env->input);
- if ( shell_env->output )
- free((void *)shell_env->output);
- free( ptr );
+ if ( shell_env->managed ) {
+ if ( shell_env->input )
+ free((void *)shell_env->input);
+ if ( shell_env->output )
+ free((void *)shell_env->output);
+ free( ptr );
+ }
}
static void rtems_shell_create_file(const char *name, const char *content)
@@ -131,6 +153,10 @@ static void rtems_shell_init_once(void)
struct passwd pwd;
struct passwd *pwd_res;
+#if SHELL_STD_DEBUG
+ default_stdout = stdout;
+#endif
+
pthread_key_create(&rtems_shell_current_env_key, rtems_shell_env_free);
/* dummy call to init /etc dir */
@@ -170,15 +196,22 @@ rtems_shell_env_t *rtems_shell_get_current_env(void)
void rtems_shell_dup_current_env(rtems_shell_env_t *copy)
{
rtems_shell_env_t *env = rtems_shell_get_current_env();
- if (env) {
+ if (env != NULL) {
*copy = *env;
}
else {
- memset(copy, 0, sizeof(rtems_shell_env_t));
- copy->magic = rtems_build_name('S', 'E', 'N', 'V');
- copy->devname = CONSOLE_DEVICE_NAME;
- copy->taskname = "RTSH";
+ *copy = rtems_global_shell_env;
+ copy->magic = rtems_build_name('S', 'E', 'N', 'V');
+ copy->devname = CONSOLE_DEVICE_NAME;
+ copy->taskname = "RTSH";
+ copy->parent_stdout = stdout;
+ copy->parent_stdin = stdin;
+ copy->parent_stderr = stderr;
}
+ /*
+ * Duplicated environments are not managed.
+ */
+ copy->managed = false;
}
/*
@@ -707,10 +740,9 @@ static bool rtems_shell_init_user_env(void)
#define RTEMS_SHELL_PROMPT_SIZE (128)
bool rtems_shell_main_loop(
- rtems_shell_env_t *shell_env_arg
+ rtems_shell_env_t *shell_env
)
{
- rtems_shell_env_t *shell_env;
int eno;
struct termios term;
struct termios previous_term;
@@ -729,9 +761,8 @@ bool rtems_shell_main_loop(
rtems_shell_init_environment();
- shell_env = rtems_shell_init_env(shell_env_arg);
- if (shell_env == NULL) {
- rtems_error(0, "rtems_shell_init_env");
+ if (shell_env->magic != rtems_build_name('S', 'E', 'N', 'V')) {
+ rtems_error(0, "invalid shell environment passed to the main loop)");
return false;
}
@@ -746,23 +777,31 @@ bool rtems_shell_main_loop(
return false;
}
- fileno(stdout);
-
- /* fprintf( stderr,
- "-%s-%s-\n", shell_env->input, shell_env->output );
- */
-
- if (shell_env->output && strcmp(shell_env->output, "stdout") != 0) {
- if (strcmp(shell_env->output, "stderr") == 0) {
- stdout = stderr;
+ shell_std_debug("shell: out: %d %d %s\n",
+ fileno(stdout), fileno(shell_env->parent_stdout),
+ shell_env->output);
+ shell_std_debug(" : in: %d %d %s\n",
+ fileno(stdin), fileno(shell_env->parent_stdin),
+ shell_env->input);
+
+ if (shell_env->output != NULL) {
+ if (strcmp(shell_env->output, "stdout") == 0) {
+ if (shell_env->parent_stdout != NULL)
+ stdout = shell_env->parent_stdout;
+ }
+ else if (strcmp(shell_env->output, "stderr") == 0) {
+ if (shell_env->parent_stderr != NULL)
+ stdout = shell_env->parent_stderr;
+ else
+ stdout = stderr;
} else if (strcmp(shell_env->output, "/dev/null") == 0) {
fclose (stdout);
} else {
- FILE *output = fopen(shell_env_arg->output,
- shell_env_arg->output_append ? "a" : "w");
- if (!output) {
+ FILE *output = fopen(shell_env->output,
+ shell_env->output_append ? "a" : "w");
+ if (output == NULL) {
fprintf(stderr, "shell: open output %s failed: %s\n",
- shell_env_arg->output, strerror(errno));
+ shell_env->output, strerror(errno));
return false;
}
stdout = output;
@@ -770,27 +809,35 @@ bool rtems_shell_main_loop(
}
}
- if (shell_env->input && strcmp(shell_env_arg->input, "stdin") != 0) {
- FILE *input = fopen(shell_env_arg->input, "r");
- if (!input) {
- fprintf(stderr, "shell: open input %s failed: %s\n",
- shell_env_arg->input, strerror(errno));
- return false;
+ if (shell_env->input != NULL) {
+ if (strcmp(shell_env->input, "stdin") == 0) {
+ if (shell_env->parent_stdin != NULL)
+ stdin = shell_env->parent_stdin;
+ } else {
+ FILE *input = fopen(shell_env->input, "r");
+ if (input == NULL) {
+ fprintf(stderr, "shell: open input %s failed: %s\n",
+ shell_env->input, strerror(errno));
+ if (stdoutToClose != NULL)
+ fclose(stdoutToClose);
+ return false;
+ }
+ stdin = input;
+ stdinToClose = input;
+ shell_env->forever = false;
+ input_file = true;
}
- stdin = input;
- stdinToClose = input;
- shell_env->forever = false;
- input_file =true;
}
- else {
- /* make a raw terminal,Linux Manuals */
+
+ if (!input_file) {
+ /* Make a raw terminal, Linux Manuals */
if (tcgetattr(fileno(stdin), &previous_term) >= 0) {
term = previous_term;
term.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
term.c_oflag &= ~OPOST;
term.c_oflag |= (OPOST|ONLCR); /* But with cr+nl on output */
term.c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
- term.c_cflag |= CLOCAL | CREAD;
+ term.c_cflag |= CLOCAL | CREAD;
term.c_cc[VMIN] = 1;
term.c_cc[VTIME] = 0;
if (tcsetattr (fileno(stdin), TCSADRAIN, &term) < 0) {
@@ -805,8 +852,13 @@ bool rtems_shell_main_loop(
"shell:cannot allocate prompt memory\n");
}
- setvbuf(stdin,NULL,_IONBF,0); /* Not buffered*/
- setvbuf(stdout,NULL,_IONBF,0); /* Not buffered*/
+ shell_std_debug("shell: child out: %d in %d\n", fileno(stdout), fileno(stdin));
+ shell_std_debug("shell: child out: %p\n", stdout);
+
+ /* Do not buffer if interactive else leave buffered */
+ if (!input_file)
+ setvbuf(stdin, NULL, _IONBF, 0);
+ setvbuf(stdout, NULL, _IONBF, 0);
/*
* Allocate the command line buffers.
@@ -869,8 +921,6 @@ bool rtems_shell_main_loop(
shell_env->exit_shell = false;
for (;;) {
- int cmd;
-
/* Prompt section */
if (prompt) {
rtems_shell_get_prompt(shell_env, prompt,
@@ -924,7 +974,11 @@ bool rtems_shell_main_loop(
memcpy (cmd_argv, cmds[cmd], RTEMS_SHELL_CMD_SIZE);
if (!rtems_shell_make_args(cmd_argv, &argc, argv,
RTEMS_SHELL_MAXIMUM_ARGUMENTS)) {
- shell_env->errorlevel = rtems_shell_execute_cmd(argv[0], argc, argv);
+ int exit_code = rtems_shell_execute_cmd(argv[0], argc, argv);
+ if (shell_env->exit_code != NULL)
+ *shell_env->exit_code = exit_code;
+ if (shell_env->exit_on_error)
+ shell_env->exit_shell = true;
}
/* end exec cmd section */
@@ -974,6 +1028,7 @@ static rtems_status_code rtems_shell_run (
const char *output,
bool output_append,
rtems_id wake_on_end,
+ int *exit_code,
bool echo,
rtems_shell_login_check_t login_check
)
@@ -983,6 +1038,8 @@ static rtems_status_code rtems_shell_run (
rtems_shell_env_t *shell_env;
rtems_name name;
+ rtems_shell_init_environment();
+
if ( task_name && strlen(task_name) >= 4)
name = rtems_build_name(
task_name[0], task_name[1], task_name[2], task_name[3]);
@@ -1008,15 +1065,21 @@ static rtems_status_code rtems_shell_run (
"allocating shell_env %s in shell_init()",task_name);
return RTEMS_NO_MEMORY;
}
+
shell_env->devname = devname;
shell_env->taskname = task_name;
+
shell_env->exit_shell = false;
shell_env->forever = forever;
shell_env->echo = echo;
- shell_env->input = strdup (input);
- shell_env->output = strdup (output);
+ shell_env->input = input == NULL ? NULL : strdup (input);
+ shell_env->output = output == NULL ? NULL : strdup (output);
shell_env->output_append = output_append;
+ shell_env->parent_stdin = stdin;
+ shell_env->parent_stdout = stdout;
+ shell_env->parent_stderr = stderr;
shell_env->wake_on_end = wake_on_end;
+ shell_env->exit_code = exit_code;
shell_env->login_check = login_check;
shell_env->uid = getuid();
shell_env->gid = getgid();
@@ -1024,9 +1087,12 @@ static rtems_status_code rtems_shell_run (
getcwd(shell_env->cwd, sizeof(shell_env->cwd));
sc = rtems_task_start(task_id, rtems_shell_task,
- (rtems_task_argument) shell_env);
+ (rtems_task_argument) shell_env);
if (sc != RTEMS_SUCCESSFUL) {
rtems_error(sc,"starting task %s in shell_init()",task_name);
+ free( (void*) shell_env->input );
+ free( (void*) shell_env->output );
+ free( shell_env );
return sc;
}
@@ -1035,7 +1101,7 @@ static rtems_status_code rtems_shell_run (
sc = rtems_event_receive (RTEMS_EVENT_1, RTEMS_WAIT, 0, &out);
}
- return 0;
+ return sc;
}
rtems_status_code rtems_shell_init(
@@ -1049,6 +1115,7 @@ rtems_status_code rtems_shell_init(
)
{
rtems_id to_wake = RTEMS_ID_NONE;
+ int exit_code = 0;
if ( wait )
to_wake = rtems_task_self();
@@ -1064,12 +1131,13 @@ rtems_status_code rtems_shell_init(
"stdout", /* output */
false, /* output_append */
to_wake, /* wake_on_end */
+ &exit_code, /* exit code of command */
false, /* echo */
login_check /* login check */
);
}
-rtems_status_code rtems_shell_script (
+rtems_status_code rtems_shell_script (
const char *task_name,
size_t task_stacksize,
rtems_task_priority task_priority,
@@ -1080,16 +1148,14 @@ rtems_status_code rtems_shell_script (
bool echo
)
{
- rtems_id current_task = RTEMS_INVALID_ID;
+ rtems_id to_wake = RTEMS_ID_NONE;
+ int exit_code = 0;
rtems_status_code sc;
- if (wait) {
- sc = rtems_task_ident (RTEMS_SELF, RTEMS_LOCAL, ¤t_task);
- if (sc != RTEMS_SUCCESSFUL)
- return sc;
- }
+ if ( wait )
+ to_wake = rtems_task_self();
- sc = rtems_shell_run(
+ return rtems_shell_run(
task_name, /* task_name */
task_stacksize, /* task_stacksize */
task_priority, /* task_priority */
@@ -1099,12 +1165,17 @@ rtems_status_code rtems_shell_script (
input, /* input */
output, /* output */
output_append, /* output_append */
- current_task, /* wake_on_end */
+ to_wake, /* wake_on_end */
+ &exit_code, /* exit_code */
echo, /* echo */
NULL /* login check */
);
- if (sc != RTEMS_SUCCESSFUL)
- return sc;
+
+ if (sc == RTEMS_SUCCESSFUL)
+ {
+ /* Place holder until RTEMS 5 is released then the interface for
+ * this call will change. */
+ }
return sc;
}
diff --git a/cpukit/libmisc/shell/shell.h b/cpukit/libmisc/shell/shell.h
index 2ab8266f27..ac8a551d7e 100644
--- a/cpukit/libmisc/shell/shell.h
+++ b/cpukit/libmisc/shell/shell.h
@@ -217,19 +217,24 @@ extern rtems_status_code rtems_shell_script(
/**
* Private environment associated with each shell instance.
*/
-typedef struct {
+typedef struct rtems_shell_env {
/** 'S','E','N','V': Shell Environment */
rtems_name magic;
+ bool managed;
const char *devname;
const char *taskname;
bool exit_shell; /* logout */
bool forever; /* repeat login */
- int errorlevel;
+ int *exit_code;
+ bool exit_on_error;
bool echo;
char cwd[256];
const char *input;
const char *output;
bool output_append;
+ FILE *parent_stdin;
+ FILE *parent_stdout;
+ FILE *parent_stderr;
rtems_id wake_on_end;
rtems_shell_login_check_t login_check;
diff --git a/testsuites/libtests/shell01/init.c b/testsuites/libtests/shell01/init.c
index b7ec2d1601..5d71c8939e 100644
--- a/testsuites/libtests/shell01/init.c
+++ b/testsuites/libtests/shell01/init.c
@@ -46,6 +46,122 @@ static void create_file(const char *name, const char *content)
rtems_test_assert(rv == 0);
}
+static const char joel_in[] =
+ "#! joel\n"
+ "jtst hello world\n"
+ "jtst 1 2 3 4 5\n";
+
+static const char joel_out_1[] =
+ " 3 'jtst hello world'\n"
+ " 6 'jtst 1 2 3 4 5'\n";
+
+static const char joel_out_2[] =
+ "\n"
+ "RTEMS Shell on (null). Use 'help' to list commands.\n"
+ " 3 'jtst hello world'\n"
+ " 6 'jtst 1 2 3 4 5'\n";
+
+static int joel_test_command(int argc, char** argv)
+{
+ int i;
+ fprintf(stdout, "%2d '", argc);
+ for (i = 0; i < argc; ++i) {
+ fprintf(stdout, argv[i]);
+ if (i < (argc - 1))
+ fprintf(stdout, " ");
+ }
+ fprintf(stdout, "'\n");
+ return 0;
+}
+
+static void test_joel(void)
+{
+ /*
+ * Running a joel script tests the shell main loop.
+ */
+ char buf[sizeof(joel_out_2) + 1];
+ rtems_shell_cmd_t* cmd;
+ FILE *in;
+ FILE *out;
+ FILE *current_stdout = stdout;
+ FILE *current_stdin = stdin;
+ size_t len;
+ rtems_status_code sc;
+
+ /*
+ * Use a private command due to the way the testsuite maps printk onto printf.
+ */
+ cmd = rtems_shell_add_cmd("jtst", "misc", "joel test", joel_test_command);
+ rtems_test_assert(cmd != NULL);
+
+ len = strlen(joel_in);
+
+ in = fopen("/jin", "w");
+ rtems_test_assert(in != NULL);
+ rtems_test_assert(fwrite(joel_in, sizeof(char), len, in) == len);
+ rtems_test_assert(fclose(in) == 0);
+
+ /*
+ * The shell opens the input and output files.
+ */
+ sc = rtems_shell_script(
+ "JTST",
+ 8 * 1024,
+ 1,
+ "/jin",
+ "/jout",
+ false,
+ true,
+ false);
+ rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+ out = fopen("/jout", "r");
+ rtems_test_assert(out != NULL);
+ rtems_test_assert(!feof(out));
+ memset(buf, 0, sizeof(buf));
+ len = fread(buf, sizeof(char), sizeof(buf), out);
+ rtems_test_assert(len > 0);
+ rtems_test_assert(strcmp(joel_out_1, buf) == 0);
+ rtems_test_assert(fclose(out) == 0);
+
+ /*
+ * The shell task inherits the parent stdin and stdout
+ */
+ in = fopen("/jin", "r");
+ rtems_test_assert(in != NULL);
+ out = fopen("/jout", "w");
+ rtems_test_assert(out != NULL);
+
+ stdin = in;
+ stdout = out;
+
+ sc = rtems_shell_script(
+ "JTST",
+ 8 * 1024,
+ 1,
+ "stdin",
+ "stdout",
+ false,
+ true,
+ false);
+ rtems_test_assert(sc == RTEMS_SUCCESSFUL);
+
+ stdout = current_stdout;
+ stdin = current_stdin;
+
+ rtems_test_assert(fclose(in) == 0);
+ rtems_test_assert(fclose(out) == 0);
+
+ out = fopen("/jout", "r");
+ rtems_test_assert(out != NULL);
+ rtems_test_assert(!feof(out));
+ memset(buf, 0, sizeof(buf));
+ len = fread(buf, sizeof(char), sizeof(buf), out);
+ rtems_test_assert(len > 0);
+ rtems_test_assert(strcmp(joel_out_2, buf) == 0);
+ rtems_test_assert(fclose(out) == 0);
+}
+
static void test(void)
{
rtems_user_env_t *uenv;
@@ -165,6 +281,7 @@ static void Init(rtems_task_argument arg)
TEST_BEGIN();
test();
+ test_joel();
TEST_END();
rtems_test_exit(0);
@@ -173,10 +290,10 @@ static void Init(rtems_task_argument arg)
#define CONFIGURE_APPLICATION_DOES_NOT_NEED_CLOCK_DRIVER
#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
-#define CONFIGURE_LIBIO_MAXIMUM_FILE_DESCRIPTORS 4
+#define CONFIGURE_LIBIO_MAXIMUM_FILE_DESCRIPTORS 5
-#define CONFIGURE_MAXIMUM_TASKS 1
-#define CONFIGURE_MAXIMUM_POSIX_KEYS 1
+#define CONFIGURE_MAXIMUM_TASKS 3
+#define CONFIGURE_MAXIMUM_POSIX_KEYS 2
#define CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS 2
#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
--
2.23.0
More information about the devel
mailing list