| Message ID | 20240716153604.22984-2-chrubis@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Shell test library v3 | expand |
Hi Cyril, I haven't finished my code review, here just comment inline to highlight errors from my local compiling. On Tue, Jul 16, 2024 at 11:35 PM Cyril Hrubis <chrubis@suse.cz> wrote: > This is a proof of a concept of a seamless C and shell integration. The > idea is that with this you can mix shell and C code as much as as you > wish to get the best of the two worlds. > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > --- > include/tst_test.h | 38 +++++++++++++ > lib/tst_test.c | 51 +++++++++++++++++ > testcases/lib/.gitignore | 1 + > testcases/lib/Makefile | 4 +- > testcases/lib/run_tests.sh | 10 ++++ > testcases/lib/tests/.gitignore | 6 ++ > testcases/lib/tests/Makefile | 11 ++++ > testcases/lib/tests/shell_loader.sh | 5 ++ > testcases/lib/tests/shell_test01.c | 17 ++++++ > testcases/lib/tests/shell_test02.c | 18 ++++++ > testcases/lib/tests/shell_test03.c | 25 +++++++++ > testcases/lib/tests/shell_test04.c | 18 ++++++ > testcases/lib/tests/shell_test05.c | 27 +++++++++ > testcases/lib/tests/shell_test06.c | 16 ++++++ > testcases/lib/tests/shell_test_brk.sh | 6 ++ > testcases/lib/tests/shell_test_check_argv.sh | 25 +++++++++ > testcases/lib/tests/shell_test_checkpoint.sh | 7 +++ > testcases/lib/tests/shell_test_pass.sh | 6 ++ > testcases/lib/tst_env.sh | 16 ++++++ > testcases/lib/tst_res_.c | 58 ++++++++++++++++++++ > 20 files changed, 363 insertions(+), 2 deletions(-) > create mode 100755 testcases/lib/run_tests.sh > create mode 100644 testcases/lib/tests/.gitignore > create mode 100644 testcases/lib/tests/Makefile > create mode 100644 testcases/lib/tests/shell_loader.sh > create mode 100644 testcases/lib/tests/shell_test01.c > create mode 100644 testcases/lib/tests/shell_test02.c > create mode 100644 testcases/lib/tests/shell_test03.c > create mode 100644 testcases/lib/tests/shell_test04.c > create mode 100644 testcases/lib/tests/shell_test05.c > create mode 100644 testcases/lib/tests/shell_test06.c > create mode 100755 testcases/lib/tests/shell_test_brk.sh > create mode 100755 testcases/lib/tests/shell_test_check_argv.sh > create mode 100755 testcases/lib/tests/shell_test_checkpoint.sh > create mode 100755 testcases/lib/tests/shell_test_pass.sh > create mode 100644 testcases/lib/tst_env.sh > create mode 100644 testcases/lib/tst_res_.c > > diff --git a/include/tst_test.h b/include/tst_test.h > index 517c8d032..fde8a1414 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -331,6 +331,8 @@ struct tst_fs { > * @child_needs_reinit: Has to be set if the test needs to call > tst_reinit() > * from a process started by exec(). > * > + * @runs_shell: Implies child_needs_reinit and forks_child at the moment. > + * > * @needs_devfs: If set the devfs is mounted at tst_test.mntpoint. This is > * needed for tests that need to create device files since > tmpfs > * at /tmp is usually mounted with 'nodev' option. > @@ -514,6 +516,7 @@ struct tst_fs { > unsigned int mount_device:1; > unsigned int needs_rofs:1; > unsigned int child_needs_reinit:1; > + unsigned int runs_shell:1; > unsigned int needs_devfs:1; > unsigned int restore_wallclock:1; > > @@ -522,6 +525,8 @@ struct tst_fs { > unsigned int skip_in_lockdown:1; > unsigned int skip_in_secureboot:1; > unsigned int skip_in_compat:1; > + > + > int needs_abi_bits; > > unsigned int needs_hugetlbfs:1; > @@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct > tst_test *self) > */ > void tst_reinit(void); > > +/** > + * tst_run_shell() - Prepare the environment and execute a shell script. > + * > + * @script_name: A filename of the script. > + * @params: A NULL terminated array of shell script parameters, pass NULL > if > + * none are needed. This what is passed starting from argv[1]. > + * > + * The shell script is executed with LTP_IPC_PATH in environment so that > the > + * binary helpers such as tst_res_ or tst_checkpoint work properly when > executed > + * from the script. This also means that the tst_test.runs_shell flag > needs to > + * be set. > + * > + * The shell script itself has to source the tst_env.sh shell script at > the > + * start and after that it's free to use tst_res in the same way C code > would > + * use. > + * > + * Example shell script that reports success:: > + * > + * #!/bin/sh > + * . tst_env.sh > + * > + * tst_res TPASS "Example test works" > + * > + * The call returns a pid in a case that you want to examine the return > value > + * of the script yourself. If you do not need to check the return value > + * yourself you can use tst_reap_children() to wait for the completion. > Or let > + * the test library collect the child automatically, just be wary that the > + * script and the test both runs concurently at the same time in this > case. > + * > + * Return: A pid of the shell process. > + */ > +int tst_run_shell(char *script_name, char *const params[]); > + > unsigned int tst_multiply_timeout(unsigned int timeout); > > /* > diff --git a/lib/tst_test.c b/lib/tst_test.c > index e5bc5bf4d..fa0907353 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -173,6 +173,52 @@ void tst_reinit(void) > SAFE_CLOSE(fd); > } > > +extern char **environ; > + > +static unsigned int params_array_len(char *const array[]) > +{ > + unsigned int ret = 0; > + > + if (!array) > + return 0; > + > + while (*(array++)) > + ret++; > + > + return ret; > +} > + > +int tst_run_shell(char *script_name, char *const params[]) > +{ > + int pid; > + unsigned int i, params_len = params_array_len(params); > + char *argv[params_len + 2] = {}; > + > + if (!tst_test->runs_shell) > + tst_brk(TBROK, "runs_shell flag must be set!"); > + > + argv[0] = script_name; > + > + if (params) { > + for (i = 0; i < params_len; i++) > + argv[i+1] = params[i]; > + } > + > + pid = SAFE_FORK(); > + if (pid) > + return pid; > + > + char *script_name_env = NULL; > + SAFE_ASPRINTF(&script_name_env, "LTP_SCRIPT_NAME=%s", script_name); > + putenv(script_name_env); > + > + execvpe(script_name, argv, environ); > Since execvpe() is a GNU extension, we need to ensure that we are compiling with GNU extensions enabled. add this line into the head of tst_test.c: #define _GNU_SOURCE + > + tst_brk(TBROK | TERRNO, "execv(%s, ...) failed!", script_name); > + > + return -1; > +} > + > static void update_results(int ttype) > { > if (!results) > @@ -1224,6 +1270,11 @@ static void do_setup(int argc, char *argv[]) > tdebug = 1; > } > > + if (tst_test->runs_shell) { > + tst_test->child_needs_reinit = 1; > + tst_test->forks_child = 1; > + } > + > if (tst_test->needs_kconfigs && > tst_kconfig_check(tst_test->needs_kconfigs)) > tst_brk(TCONF, "Aborting due to unsuitable kernel config, > see above!"); > > diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore > index e8afd06f3..d0dacf62a 100644 > --- a/testcases/lib/.gitignore > +++ b/testcases/lib/.gitignore > @@ -23,3 +23,4 @@ > /tst_sleep > /tst_supported_fs > /tst_timeout_kill > +/tst_res_ > diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile > index 990b46089..928d76d62 100644 > --- a/testcases/lib/Makefile > +++ b/testcases/lib/Makefile > @@ -13,6 +13,6 @@ MAKE_TARGETS := tst_sleep tst_random > tst_checkpoint tst_rod tst_kvcmp\ > tst_getconf tst_supported_fs tst_check_drivers > tst_get_unused_port\ > tst_get_median tst_hexdump tst_get_free_pids > tst_timeout_kill\ > tst_check_kconfigs tst_cgctl tst_fsfreeze > tst_ns_create tst_ns_exec\ > - tst_ns_ifmove tst_lockdown_enabled > tst_secureboot_enabled > + tst_ns_ifmove tst_lockdown_enabled > tst_secureboot_enabled tst_res_ > > -include $(top_srcdir)/include/mk/generic_leaf_target.mk > +include $(top_srcdir)/include/mk/generic_trunk_target.mk > diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh > new file mode 100755 > index 000000000..88607b146 > --- /dev/null > +++ b/testcases/lib/run_tests.sh > @@ -0,0 +1,10 @@ > +#!/bin/sh > + > +export PATH=$PATH:$PWD:$PWD/tests/ > + > +./tests/shell_test01 > +./tests/shell_test02 > +./tests/shell_test03 > +./tests/shell_test04 > +./tests/shell_test05 > +./tests/shell_test06 > diff --git a/testcases/lib/tests/.gitignore > b/testcases/lib/tests/.gitignore > new file mode 100644 > index 000000000..da967c4d6 > --- /dev/null > +++ b/testcases/lib/tests/.gitignore > @@ -0,0 +1,6 @@ > +shell_test01 > +shell_test02 > +shell_test03 > +shell_test04 > +shell_test05 > +shell_test06 > diff --git a/testcases/lib/tests/Makefile b/testcases/lib/tests/Makefile > new file mode 100644 > index 000000000..5a5cf5310 > --- /dev/null > +++ b/testcases/lib/tests/Makefile > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (C) 2009, Cisco Systems Inc. > +# Ngie Cooper, August 2009 > + > +top_srcdir ?= ../../.. > + > +include $(top_srcdir)/include/mk/testcases.mk > + > +INSTALL_TARGETS= > + > +include $(top_srcdir)/include/mk/generic_leaf_target.mk > diff --git a/testcases/lib/tests/shell_loader.sh > b/testcases/lib/tests/shell_loader.sh > new file mode 100644 > index 000000000..c3b3cf5fd > --- /dev/null > +++ b/testcases/lib/tests/shell_loader.sh > @@ -0,0 +1,5 @@ > +#!/usr/bin/env tst_run_shell > + > +. tst_env.sh > + > +tst_res TPASS "Shell loader works fine!" > diff --git a/testcases/lib/tests/shell_test01.c > b/testcases/lib/tests/shell_test01.c > new file mode 100644 > index 000000000..18f834138 > --- /dev/null > +++ b/testcases/lib/tests/shell_test01.c > @@ -0,0 +1,17 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + tst_run_shell("shell_test_pass.sh", NULL); > + tst_res(TINFO, "C test exits now"); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test02.c > b/testcases/lib/tests/shell_test02.c > new file mode 100644 > index 000000000..3b0534370 > --- /dev/null > +++ b/testcases/lib/tests/shell_test02.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + tst_run_shell("shell_test_pass.sh", NULL); > + tst_reap_children(); > + tst_res(TINFO, "Shell test has finished at this point!"); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test03.c > b/testcases/lib/tests/shell_test03.c > new file mode 100644 > index 000000000..c11a09e4e > --- /dev/null > +++ b/testcases/lib/tests/shell_test03.c > @@ -0,0 +1,25 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include <sys/wait.h> > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + int pid, status; > + > + pid = tst_run_shell("shell_test_pass.sh", NULL); > + > + tst_res(TINFO, "Waiting for the pid %i", pid); > + > + waitpid(pid, &status, 0); > + > + tst_res(TINFO, "Shell test has %s", tst_strstatus(status)); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test04.c > b/testcases/lib/tests/shell_test04.c > new file mode 100644 > index 000000000..fd1fd122c > --- /dev/null > +++ b/testcases/lib/tests/shell_test04.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + char *const params[] = {"param1", "param2", NULL}; > + > + tst_run_shell("shell_test_check_argv.sh", params); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test05.c > b/testcases/lib/tests/shell_test05.c > new file mode 100644 > index 000000000..2141d4790 > --- /dev/null > +++ b/testcases/lib/tests/shell_test05.c > @@ -0,0 +1,27 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + int pid; > + > + pid = tst_run_shell("shell_test_checkpoint.sh", NULL); > + > + tst_res(TINFO, "Waiting for shell to sleep on checkpoint!"); > + > + TST_PROCESS_STATE_WAIT(pid, 'S', 10000); > + > + tst_res(TINFO, "Waking shell child!"); > + > + TST_CHECKPOINT_WAKE(0); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .needs_checkpoints = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test06.c > b/testcases/lib/tests/shell_test06.c > new file mode 100644 > index 000000000..25abc1e7b > --- /dev/null > +++ b/testcases/lib/tests/shell_test06.c > @@ -0,0 +1,16 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + tst_run_shell("shell_test_brk.sh", NULL); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test_brk.sh > b/testcases/lib/tests/shell_test_brk.sh > new file mode 100755 > index 000000000..f266dc3fe > --- /dev/null > +++ b/testcases/lib/tests/shell_test_brk.sh > @@ -0,0 +1,6 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +tst_brk TCONF "This exits test and the next message should not be reached" > +tst_res TFAIL "If you see this the test failed" > diff --git a/testcases/lib/tests/shell_test_check_argv.sh > b/testcases/lib/tests/shell_test_check_argv.sh > new file mode 100755 > index 000000000..756189ee7 > --- /dev/null > +++ b/testcases/lib/tests/shell_test_check_argv.sh > @@ -0,0 +1,25 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +echo "$@" > + > +tst_res TINFO "argv = $@" > + > +if [ $# -ne 2 ]; then > + tst_res TFAIL "Wrong number of parameters got $# expected 2" > +else > + tst_res TPASS "Got 2 parameters" > +fi > + > +if [ "$1" != "param1" ]; then > + tst_res TFAIL "First parameter is $1 expected param1" > +else > + tst_res TPASS "First parameter is $1" > +fi > + > +if [ "$2" != "param2" ]; then > + tst_res TFAIL "Second parameter is $2 expected param2" > +else > + tst_res TPASS "Second parameter is $2" > +fi > diff --git a/testcases/lib/tests/shell_test_checkpoint.sh > b/testcases/lib/tests/shell_test_checkpoint.sh > new file mode 100755 > index 000000000..0ceb7cf66 > --- /dev/null > +++ b/testcases/lib/tests/shell_test_checkpoint.sh > @@ -0,0 +1,7 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +tst_res TINFO "Waiting for a checkpoint 0" > +tst_checkpoint wait 10000 0 > +tst_res TPASS "Continuing after checkpoint" > diff --git a/testcases/lib/tests/shell_test_pass.sh > b/testcases/lib/tests/shell_test_pass.sh > new file mode 100755 > index 000000000..fd0684eb2 > --- /dev/null > +++ b/testcases/lib/tests/shell_test_pass.sh > @@ -0,0 +1,6 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +tst_res TPASS "This is called from the shell script!" > +tst_sleep 100ms > diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh > new file mode 100644 > index 000000000..c8f3c2160 > --- /dev/null > +++ b/testcases/lib/tst_env.sh > @@ -0,0 +1,16 @@ > +#!/bin/sh > + > +tst_script_name=$(basename $0) > + > +tst_brk_() > +{ > + tst_res_ $@ > + > + case "$3" in > + "TBROK") exit 2;; > + *) exit 0;; > + esac > +} > + > +alias tst_res="tst_res_ $tst_script_name \$LINENO" > +alias tst_brk="tst_brk_ $tst_script_name \$LINENO" > diff --git a/testcases/lib/tst_res_.c b/testcases/lib/tst_res_.c > new file mode 100644 > index 000000000..cc3fa53d3 > --- /dev/null > +++ b/testcases/lib/tst_res_.c > @@ -0,0 +1,58 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz> > + */ > + > +#define TST_NO_DEFAULT_MAIN > +#include "tst_test.h" > + > +static void print_help(void) > +{ > + printf("Usage: tst_res_ filename lineno > [TPASS|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n"); > +} > + > +int main(int argc, char *argv[]) > +{ > + int type, i; > + > + if (argc < 5) > + goto help; > + > + if (!strcmp(argv[3], "TPASS")) > + type = TPASS; > + else if (!strcmp(argv[3], "TFAIL")) > + type = TFAIL; > + else if (!strcmp(argv[3], "TCONF")) > + type = TCONF; > + else if (!strcmp(argv[3], "TINFO")) > + type = TINFO; > + else if (!strcmp(argv[3], "TDEBUG")) > + type = TDEBUG; > + else > + goto help; > + > + size_t len = 0; > + > + for (i = 4; i < argc; i++) > + len += strlen(argv[i]) + 1; > + > + char *msg = SAFE_MALLOC(len); > + char *msgp = msg; > + > + for (i = 4; i < argc; i++) { > + msgp = strcpy(msgp, argv[i]); > + msgp += strlen(argv[i]); > + *(msgp++) = ' '; > + } > + > + *(msgp - 1) = 0; > + > + tst_reinit(); > + > + tst_res_(argv[1], atoi(argv[2]), type, msg); > tst_res_.c:52:9: error: format not a string literal and no format arguments [-Werror=format-security] 52 | tst_res_(argv[1], atoi(argv[2]), type, msg); | ^~~~~~~~ cc1: some warnings being treated as errors make: *** [../../include/mk/rules.mk:45: tst_res_] Error 1 > + > + return 0; > +help: > + print_help(); > + return 1; > +} > -- > 2.44.2 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > >
Hi! > tst_res_.c:52:9: error: format not a string literal and no format arguments > [-Werror=format-security] > 52 | tst_res_(argv[1], atoi(argv[2]), type, msg); > | ^~~~~~~~ Right, this needs: tst_res_(argv[1], atoi(argv[2]), type, "%s", msg); I wonder why I didn't get warning at my end.
Hi! > Since execvpe() is a GNU extension, we need to ensure that > we are compiling with GNU extensions enabled. > > add this line into the head of tst_test.c: > #define _GNU_SOURCE As for the execvpe() I've used that for the prototype but I'm unsure if it could be used in the final product, since this is the test library it has to compile on all kinds of libc out there. It looks like musl does support it but I haven't checked Android bionic. So we may need to write our own implemtantion on the top of the execve() syscall. But that should be as easy as getting the path to the script before we pass it to execve().
On Wed, Jul 17, 2024 at 4:38 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > tst_res_.c:52:9: error: format not a string literal and no format > arguments > > [-Werror=format-security] > > 52 | tst_res_(argv[1], atoi(argv[2]), type, msg); > > | ^~~~~~~~ > > Right, this needs: > > tst_res_(argv[1], atoi(argv[2]), type, "%s", msg); > > I wonder why I didn't get warning at my end. > Probably because my GCC version is newer: gcc version 14.1.1 20240607 (Red Hat 14.1.1-5) (GCC)
On Wed, Jul 17, 2024 at 4:49 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > Since execvpe() is a GNU extension, we need to ensure that > > we are compiling with GNU extensions enabled. > > > > add this line into the head of tst_test.c: > > #define _GNU_SOURCE > > As for the execvpe() I've used that for the prototype but I'm unsure if > it could be used in the final product, since this is the test library it > has to compile on all kinds of libc out there. It looks like musl does > support it but I haven't checked Android bionic. > So we may need to write our own implemtantion on the top of the execve() > syscall. But that should be as easy as getting the path to the script > before we pass it to execve(). > Indeed. We need to create a ltp_execvpe() based on execve().
Hi all, > On Wed, Jul 17, 2024 at 4:49 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > Since execvpe() is a GNU extension, we need to ensure that > > > we are compiling with GNU extensions enabled. > > > add this line into the head of tst_test.c: > > > #define _GNU_SOURCE Good, you already noticed. > > As for the execvpe() I've used that for the prototype but I'm unsure if > > it could be used in the final product, since this is the test library it > > has to compile on all kinds of libc out there. It looks like musl does > > support it but I haven't checked Android bionic. > > So we may need to write our own implemtantion on the top of the execve() > > syscall. But that should be as easy as getting the path to the script > > before we pass it to execve(). It supports it. It looks like even without _GNU_SOURCE: https://android.googlesource.com/platform/bionic.git/+/refs/heads/main/libc/include/unistd.h#142 But better to keep Edward in loop. > Indeed. We need to create a ltp_execvpe() based on execve(). So there will be v2, right? Kind regards, Petr
Hi all, > On Wed, Jul 17, 2024 at 4:49 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > Since execvpe() is a GNU extension, we need to ensure that > > > we are compiling with GNU extensions enabled. > > > add this line into the head of tst_test.c: > > > #define _GNU_SOURCE FYI besides this (which brings CI build) it fails to compile also on musl: https://github.com/pevik/ltp/actions/runs/9978962369/job/27577025071 tst_res_.c: In function 'main': tst_res_.c:52:9: error: format not a string literal and no format arguments [-Werror=format-security] 52 | tst_res_(argv[1], atoi(argv[2]), type, msg); Kind regards, Petr
Hi! > tst_res_.c: In function 'main': > tst_res_.c:52:9: error: format not a string literal and no format arguments [-Werror=format-security] > 52 | tst_res_(argv[1], atoi(argv[2]), type, msg); Already reported by Li, it's missing the format string "%s" before the msg.
Hi all,
one more thing to fix (or did I overlook the discussion)?
tst_test.c: In function ‘tst_run_shell’:
tst_test.c:196:2: error: variable-sized object may not be initialized
char *argv[params_len + 2] = {};
^~~~
Found on gcc in SLES 15-SP6:
gcc --version
gcc (SUSE Linux) 7.5.0
Kind regards,
Petr
Hi Cyril, > This is a proof of a concept of a seamless C and shell integration. The > idea is that with this you can mix shell and C code as much as as you > wish to get the best of the two worlds. > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > --- > include/tst_test.h | 38 +++++++++++++ > lib/tst_test.c | 51 +++++++++++++++++ > testcases/lib/.gitignore | 1 + > testcases/lib/Makefile | 4 +- > testcases/lib/run_tests.sh | 10 ++++ > testcases/lib/tests/.gitignore | 6 ++ > testcases/lib/tests/Makefile | 11 ++++ > testcases/lib/tests/shell_loader.sh | 5 ++ > testcases/lib/tests/shell_test01.c | 17 ++++++ > testcases/lib/tests/shell_test02.c | 18 ++++++ > testcases/lib/tests/shell_test03.c | 25 +++++++++ > testcases/lib/tests/shell_test04.c | 18 ++++++ > testcases/lib/tests/shell_test05.c | 27 +++++++++ > testcases/lib/tests/shell_test06.c | 16 ++++++ FYI we have shell tests for new library in lib/newlib_tests (C tests) and lib/newlib_tests/shell/ (shell tests), is it necessary to add new location? Or, if you prefer this, we should move existing tests from lib/newlib_tests/shell/ to this new location. Also, we have lib/newlib_tests/runtest.sh script, which runs currently only te tests which exit 0 (TPASS or TCONF). Here are LTP_SHELL_API_TESTS listed. Also we have test-shell target in the top level Makefile. => these tests (currently only these which exits 0) should be run by test-shell and thus in CI. Generally the approach LGTM. > unsigned int needs_hugetlbfs:1; > @@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self) > */ > void tst_reinit(void); > +/** > + * tst_run_shell() - Prepare the environment and execute a shell script. > + * > + * @script_name: A filename of the script. > + * @params: A NULL terminated array of shell script parameters, pass NULL if > + * none are needed. This what is passed starting from argv[1]. > + * > + * The shell script is executed with LTP_IPC_PATH in environment so that the > + * binary helpers such as tst_res_ or tst_checkpoint work properly when executed > + * from the script. This also means that the tst_test.runs_shell flag needs to > + * be set. > + * > + * The shell script itself has to source the tst_env.sh shell script at the > + * start and after that it's free to use tst_res in the same way C code would > + * use. > + * > + * Example shell script that reports success:: nit: double : > + * > + * #!/bin/sh > + * . tst_env.sh > + * > + * tst_res TPASS "Example test works" > + * > + * The call returns a pid in a case that you want to examine the return value > + * of the script yourself. If you do not need to check the return value > + * yourself you can use tst_reap_children() to wait for the completion. Or let > + * the test library collect the child automatically, just be wary that the > + * script and the test both runs concurently at the same time in this case. > + * > + * Return: A pid of the shell process. > + */ > +int tst_run_shell(char *script_name, char *const params[]); > + > unsigned int tst_multiply_timeout(unsigned int timeout); > /* > diff --git a/lib/tst_test.c b/lib/tst_test.c > index e5bc5bf4d..fa0907353 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -173,6 +173,52 @@ void tst_reinit(void) > SAFE_CLOSE(fd); > } ... > +int tst_run_shell(char *script_name, char *const params[]) > +{ > + int pid; > + unsigned int i, params_len = params_array_len(params); > + char *argv[params_len + 2] = {}; As I noted, this is a problem for old gcc 7 (still used in 15-SP6). Kind regards, Petr
Hi! > one more thing to fix (or did I overlook the discussion)? > > tst_test.c: In function ‘tst_run_shell’: > tst_test.c:196:2: error: variable-sized object may not be initialized > char *argv[params_len + 2] = {}; > ^~~~ That's a new one, I will fix it in v2.
Hi! > > This is a proof of a concept of a seamless C and shell integration. The > > idea is that with this you can mix shell and C code as much as as you > > wish to get the best of the two worlds. > > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > > --- > > include/tst_test.h | 38 +++++++++++++ > > lib/tst_test.c | 51 +++++++++++++++++ > > testcases/lib/.gitignore | 1 + > > testcases/lib/Makefile | 4 +- > > testcases/lib/run_tests.sh | 10 ++++ > > testcases/lib/tests/.gitignore | 6 ++ > > testcases/lib/tests/Makefile | 11 ++++ > > testcases/lib/tests/shell_loader.sh | 5 ++ > > testcases/lib/tests/shell_test01.c | 17 ++++++ > > testcases/lib/tests/shell_test02.c | 18 ++++++ > > testcases/lib/tests/shell_test03.c | 25 +++++++++ > > testcases/lib/tests/shell_test04.c | 18 ++++++ > > testcases/lib/tests/shell_test05.c | 27 +++++++++ > > testcases/lib/tests/shell_test06.c | 16 ++++++ > FYI we have shell tests for new library in lib/newlib_tests (C tests) and > lib/newlib_tests/shell/ (shell tests), is it necessary to add new location? Or, > if you prefer this, we should move existing tests from lib/newlib_tests/shell/ > to this new location. For a historical reasons the lib for shell is in testcases/lib/ and the tests use paths to binaries and scripts in there so I added the code there. We may as well move it to the top level lib/shell, or create top level slib (as for shell lib) or anywhere else as long as we agree on a better place to put the code.
> Hi! > > > This is a proof of a concept of a seamless C and shell integration. The > > > idea is that with this you can mix shell and C code as much as as you > > > wish to get the best of the two worlds. > > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > > > --- > > > include/tst_test.h | 38 +++++++++++++ > > > lib/tst_test.c | 51 +++++++++++++++++ > > > testcases/lib/.gitignore | 1 + > > > testcases/lib/Makefile | 4 +- > > > testcases/lib/run_tests.sh | 10 ++++ > > > testcases/lib/tests/.gitignore | 6 ++ > > > testcases/lib/tests/Makefile | 11 ++++ > > > testcases/lib/tests/shell_loader.sh | 5 ++ > > > testcases/lib/tests/shell_test01.c | 17 ++++++ > > > testcases/lib/tests/shell_test02.c | 18 ++++++ > > > testcases/lib/tests/shell_test03.c | 25 +++++++++ > > > testcases/lib/tests/shell_test04.c | 18 ++++++ > > > testcases/lib/tests/shell_test05.c | 27 +++++++++ > > > testcases/lib/tests/shell_test06.c | 16 ++++++ > > FYI we have shell tests for new library in lib/newlib_tests (C tests) and > > lib/newlib_tests/shell/ (shell tests), is it necessary to add new location? Or, > > if you prefer this, we should move existing tests from lib/newlib_tests/shell/ > > to this new location. > For a historical reasons the lib for shell is in testcases/lib/ and the > tests use paths to binaries and scripts in there so I added the code > there. We may as well move it to the top level lib/shell, or create top > level slib (as for shell lib) or anywhere else as long as we agree on a > better place to put the code. Yeah, we could move everything to lib/shell. I'm also OK with different location with the tests, i.e. we can keep things where they are. My main concern is to run the tests in CI and in 'make test' target (also specify new subtest e.g. test-c-shell: or add them to test-c). Kind regards, Petr
Hi, I'll skip bugs reported by others but I have some comments below. On 16. 07. 24 17:36, Cyril Hrubis wrote: > This is a proof of a concept of a seamless C and shell integration. The > idea is that with this you can mix shell and C code as much as as you > wish to get the best of the two worlds. > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > --- > include/tst_test.h | 38 +++++++++++++ > lib/tst_test.c | 51 +++++++++++++++++ > testcases/lib/.gitignore | 1 + > testcases/lib/Makefile | 4 +- > testcases/lib/run_tests.sh | 10 ++++ > testcases/lib/tests/.gitignore | 6 ++ > testcases/lib/tests/Makefile | 11 ++++ > testcases/lib/tests/shell_loader.sh | 5 ++ > testcases/lib/tests/shell_test01.c | 17 ++++++ > testcases/lib/tests/shell_test02.c | 18 ++++++ > testcases/lib/tests/shell_test03.c | 25 +++++++++ > testcases/lib/tests/shell_test04.c | 18 ++++++ > testcases/lib/tests/shell_test05.c | 27 +++++++++ > testcases/lib/tests/shell_test06.c | 16 ++++++ > testcases/lib/tests/shell_test_brk.sh | 6 ++ > testcases/lib/tests/shell_test_check_argv.sh | 25 +++++++++ > testcases/lib/tests/shell_test_checkpoint.sh | 7 +++ > testcases/lib/tests/shell_test_pass.sh | 6 ++ > testcases/lib/tst_env.sh | 16 ++++++ > testcases/lib/tst_res_.c | 58 ++++++++++++++++++++ > 20 files changed, 363 insertions(+), 2 deletions(-) > create mode 100755 testcases/lib/run_tests.sh > create mode 100644 testcases/lib/tests/.gitignore > create mode 100644 testcases/lib/tests/Makefile > create mode 100644 testcases/lib/tests/shell_loader.sh > create mode 100644 testcases/lib/tests/shell_test01.c > create mode 100644 testcases/lib/tests/shell_test02.c > create mode 100644 testcases/lib/tests/shell_test03.c > create mode 100644 testcases/lib/tests/shell_test04.c > create mode 100644 testcases/lib/tests/shell_test05.c > create mode 100644 testcases/lib/tests/shell_test06.c > create mode 100755 testcases/lib/tests/shell_test_brk.sh > create mode 100755 testcases/lib/tests/shell_test_check_argv.sh > create mode 100755 testcases/lib/tests/shell_test_checkpoint.sh > create mode 100755 testcases/lib/tests/shell_test_pass.sh > create mode 100644 testcases/lib/tst_env.sh > create mode 100644 testcases/lib/tst_res_.c > > diff --git a/include/tst_test.h b/include/tst_test.h > index 517c8d032..fde8a1414 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -331,6 +331,8 @@ struct tst_fs { > * @child_needs_reinit: Has to be set if the test needs to call tst_reinit() > * from a process started by exec(). > * > + * @runs_shell: Implies child_needs_reinit and forks_child at the moment. > + * This could also work as is for Python or Perl scripts (which we don't support at the moment) but consider renaming the attribute to .runs_script anyway. > * @needs_devfs: If set the devfs is mounted at tst_test.mntpoint. This is > * needed for tests that need to create device files since tmpfs > * at /tmp is usually mounted with 'nodev' option. > @@ -514,6 +516,7 @@ struct tst_fs { > unsigned int mount_device:1; > unsigned int needs_rofs:1; > unsigned int child_needs_reinit:1; > + unsigned int runs_shell:1; > unsigned int needs_devfs:1; > unsigned int restore_wallclock:1; > > @@ -522,6 +525,8 @@ struct tst_fs { > unsigned int skip_in_lockdown:1; > unsigned int skip_in_secureboot:1; > unsigned int skip_in_compat:1; > + > + > int needs_abi_bits; > > unsigned int needs_hugetlbfs:1; > @@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self) > */ > void tst_reinit(void); > > +/** > + * tst_run_shell() - Prepare the environment and execute a shell script. > + * > + * @script_name: A filename of the script. > + * @params: A NULL terminated array of shell script parameters, pass NULL if > + * none are needed. This what is passed starting from argv[1]. > + * > + * The shell script is executed with LTP_IPC_PATH in environment so that the > + * binary helpers such as tst_res_ or tst_checkpoint work properly when executed > + * from the script. This also means that the tst_test.runs_shell flag needs to > + * be set. > + * > + * The shell script itself has to source the tst_env.sh shell script at the > + * start and after that it's free to use tst_res in the same way C code would > + * use. > + * > + * Example shell script that reports success:: > + * > + * #!/bin/sh > + * . tst_env.sh > + * > + * tst_res TPASS "Example test works" > + * > + * The call returns a pid in a case that you want to examine the return value > + * of the script yourself. If you do not need to check the return value > + * yourself you can use tst_reap_children() to wait for the completion. Or let > + * the test library collect the child automatically, just be wary that the > + * script and the test both runs concurently at the same time in this case. > + * > + * Return: A pid of the shell process. > + */ > +int tst_run_shell(char *script_name, char *const params[]); > + > unsigned int tst_multiply_timeout(unsigned int timeout); > > /* > diff --git a/lib/tst_test.c b/lib/tst_test.c > index e5bc5bf4d..fa0907353 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -173,6 +173,52 @@ void tst_reinit(void) > SAFE_CLOSE(fd); > } > > +extern char **environ; > + > +static unsigned int params_array_len(char *const array[]) > +{ > + unsigned int ret = 0; > + > + if (!array) > + return 0; > + > + while (*(array++)) > + ret++; > + > + return ret; > +} > + > +int tst_run_shell(char *script_name, char *const params[]) > +{ > + int pid; > + unsigned int i, params_len = params_array_len(params); > + char *argv[params_len + 2] = {}; > + > + if (!tst_test->runs_shell) > + tst_brk(TBROK, "runs_shell flag must be set!"); > + > + argv[0] = script_name; > + > + if (params) { > + for (i = 0; i < params_len; i++) > + argv[i+1] = params[i]; > + } > + > + pid = SAFE_FORK(); > + if (pid) > + return pid; > + > + char *script_name_env = NULL; > + SAFE_ASPRINTF(&script_name_env, "LTP_SCRIPT_NAME=%s", script_name); > + putenv(script_name_env); > + > + execvpe(script_name, argv, environ); > + > + tst_brk(TBROK | TERRNO, "execv(%s, ...) failed!", script_name); > + > + return -1; > +} > + > static void update_results(int ttype) > { > if (!results) > @@ -1224,6 +1270,11 @@ static void do_setup(int argc, char *argv[]) > tdebug = 1; > } > > + if (tst_test->runs_shell) { > + tst_test->child_needs_reinit = 1; > + tst_test->forks_child = 1; > + } > + > if (tst_test->needs_kconfigs && tst_kconfig_check(tst_test->needs_kconfigs)) > tst_brk(TCONF, "Aborting due to unsuitable kernel config, see above!"); > > diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore > index e8afd06f3..d0dacf62a 100644 > --- a/testcases/lib/.gitignore > +++ b/testcases/lib/.gitignore > @@ -23,3 +23,4 @@ > /tst_sleep > /tst_supported_fs > /tst_timeout_kill > +/tst_res_ > diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile > index 990b46089..928d76d62 100644 > --- a/testcases/lib/Makefile > +++ b/testcases/lib/Makefile > @@ -13,6 +13,6 @@ MAKE_TARGETS := tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\ > tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\ > tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\ > tst_check_kconfigs tst_cgctl tst_fsfreeze tst_ns_create tst_ns_exec\ > - tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled > + tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_ > > -include $(top_srcdir)/include/mk/generic_leaf_target.mk > +include $(top_srcdir)/include/mk/generic_trunk_target.mk > diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh > new file mode 100755 > index 000000000..88607b146 > --- /dev/null > +++ b/testcases/lib/run_tests.sh > @@ -0,0 +1,10 @@ > +#!/bin/sh > + > +export PATH=$PATH:$PWD:$PWD/tests/ $PWD is not the correct path. It should be set as follows: testdir=$(dirname $0) export PATH=$PATH:$testdir:$testdir/tests/ > + > +./tests/shell_test01 > +./tests/shell_test02 > +./tests/shell_test03 > +./tests/shell_test04 > +./tests/shell_test05 > +./tests/shell_test06 > diff --git a/testcases/lib/tests/.gitignore b/testcases/lib/tests/.gitignore > new file mode 100644 > index 000000000..da967c4d6 > --- /dev/null > +++ b/testcases/lib/tests/.gitignore > @@ -0,0 +1,6 @@ > +shell_test01 > +shell_test02 > +shell_test03 > +shell_test04 > +shell_test05 > +shell_test06 > diff --git a/testcases/lib/tests/Makefile b/testcases/lib/tests/Makefile > new file mode 100644 > index 000000000..5a5cf5310 > --- /dev/null > +++ b/testcases/lib/tests/Makefile > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (C) 2009, Cisco Systems Inc. > +# Ngie Cooper, August 2009 > + > +top_srcdir ?= ../../.. > + > +include $(top_srcdir)/include/mk/testcases.mk > + > +INSTALL_TARGETS= > + > +include $(top_srcdir)/include/mk/generic_leaf_target.mk > diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh > new file mode 100644 > index 000000000..c3b3cf5fd > --- /dev/null > +++ b/testcases/lib/tests/shell_loader.sh > @@ -0,0 +1,5 @@ > +#!/usr/bin/env tst_run_shell > + > +. tst_env.sh > + > +tst_res TPASS "Shell loader works fine!" > diff --git a/testcases/lib/tests/shell_test01.c b/testcases/lib/tests/shell_test01.c > new file mode 100644 > index 000000000..18f834138 > --- /dev/null > +++ b/testcases/lib/tests/shell_test01.c > @@ -0,0 +1,17 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + tst_run_shell("shell_test_pass.sh", NULL); > + tst_res(TINFO, "C test exits now"); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test02.c b/testcases/lib/tests/shell_test02.c > new file mode 100644 > index 000000000..3b0534370 > --- /dev/null > +++ b/testcases/lib/tests/shell_test02.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + tst_run_shell("shell_test_pass.sh", NULL); > + tst_reap_children(); > + tst_res(TINFO, "Shell test has finished at this point!"); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test03.c b/testcases/lib/tests/shell_test03.c > new file mode 100644 > index 000000000..c11a09e4e > --- /dev/null > +++ b/testcases/lib/tests/shell_test03.c > @@ -0,0 +1,25 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include <sys/wait.h> > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + int pid, status; > + > + pid = tst_run_shell("shell_test_pass.sh", NULL); > + > + tst_res(TINFO, "Waiting for the pid %i", pid); > + > + waitpid(pid, &status, 0); > + > + tst_res(TINFO, "Shell test has %s", tst_strstatus(status)); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test04.c b/testcases/lib/tests/shell_test04.c > new file mode 100644 > index 000000000..fd1fd122c > --- /dev/null > +++ b/testcases/lib/tests/shell_test04.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + char *const params[] = {"param1", "param2", NULL}; > + > + tst_run_shell("shell_test_check_argv.sh", params); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test05.c b/testcases/lib/tests/shell_test05.c > new file mode 100644 > index 000000000..2141d4790 > --- /dev/null > +++ b/testcases/lib/tests/shell_test05.c > @@ -0,0 +1,27 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + int pid; > + > + pid = tst_run_shell("shell_test_checkpoint.sh", NULL); > + > + tst_res(TINFO, "Waiting for shell to sleep on checkpoint!"); > + > + TST_PROCESS_STATE_WAIT(pid, 'S', 10000); > + > + tst_res(TINFO, "Waking shell child!"); > + > + TST_CHECKPOINT_WAKE(0); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .needs_checkpoints = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test06.c b/testcases/lib/tests/shell_test06.c > new file mode 100644 > index 000000000..25abc1e7b > --- /dev/null > +++ b/testcases/lib/tests/shell_test06.c > @@ -0,0 +1,16 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Shell test example. > + */ > + > +#include "tst_test.h" > + > +static void run_test(void) > +{ > + tst_run_shell("shell_test_brk.sh", NULL); > +} > + > +static struct tst_test test = { > + .runs_shell = 1, > + .test_all = run_test, > +}; > diff --git a/testcases/lib/tests/shell_test_brk.sh b/testcases/lib/tests/shell_test_brk.sh > new file mode 100755 > index 000000000..f266dc3fe > --- /dev/null > +++ b/testcases/lib/tests/shell_test_brk.sh > @@ -0,0 +1,6 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +tst_brk TCONF "This exits test and the next message should not be reached" > +tst_res TFAIL "If you see this the test failed" > diff --git a/testcases/lib/tests/shell_test_check_argv.sh b/testcases/lib/tests/shell_test_check_argv.sh > new file mode 100755 > index 000000000..756189ee7 > --- /dev/null > +++ b/testcases/lib/tests/shell_test_check_argv.sh > @@ -0,0 +1,25 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +echo "$@" > + > +tst_res TINFO "argv = $@" > + > +if [ $# -ne 2 ]; then > + tst_res TFAIL "Wrong number of parameters got $# expected 2" > +else > + tst_res TPASS "Got 2 parameters" > +fi > + > +if [ "$1" != "param1" ]; then > + tst_res TFAIL "First parameter is $1 expected param1" > +else > + tst_res TPASS "First parameter is $1" > +fi > + > +if [ "$2" != "param2" ]; then > + tst_res TFAIL "Second parameter is $2 expected param2" > +else > + tst_res TPASS "Second parameter is $2" > +fi > diff --git a/testcases/lib/tests/shell_test_checkpoint.sh b/testcases/lib/tests/shell_test_checkpoint.sh > new file mode 100755 > index 000000000..0ceb7cf66 > --- /dev/null > +++ b/testcases/lib/tests/shell_test_checkpoint.sh > @@ -0,0 +1,7 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +tst_res TINFO "Waiting for a checkpoint 0" > +tst_checkpoint wait 10000 0 > +tst_res TPASS "Continuing after checkpoint" > diff --git a/testcases/lib/tests/shell_test_pass.sh b/testcases/lib/tests/shell_test_pass.sh > new file mode 100755 > index 000000000..fd0684eb2 > --- /dev/null > +++ b/testcases/lib/tests/shell_test_pass.sh > @@ -0,0 +1,6 @@ > +#!/bin/sh > + > +. tst_env.sh > + > +tst_res TPASS "This is called from the shell script!" > +tst_sleep 100ms > diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh > new file mode 100644 > index 000000000..c8f3c2160 > --- /dev/null > +++ b/testcases/lib/tst_env.sh > @@ -0,0 +1,16 @@ > +#!/bin/sh > + > +tst_script_name=$(basename $0) > + > +tst_brk_() > +{ > + tst_res_ $@ > + > + case "$3" in > + "TBROK") exit 2;; > + *) exit 0;; The exit command above will ignore any other result flags already set during the test. You could modify tst_res_ to return the correct exit code and then call it like this: tst_brk() { tst_res_ "$@" exit $? } Also note the quotation marks around $@. Without them, tst_brk will compress whitespace which the caller wants to preserve. > + esac > +} > + > +alias tst_res="tst_res_ $tst_script_name \$LINENO" > +alias tst_brk="tst_brk_ $tst_script_name \$LINENO" > diff --git a/testcases/lib/tst_res_.c b/testcases/lib/tst_res_.c > new file mode 100644 > index 000000000..cc3fa53d3 > --- /dev/null > +++ b/testcases/lib/tst_res_.c > @@ -0,0 +1,58 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz> > + */ > + > +#define TST_NO_DEFAULT_MAIN > +#include "tst_test.h" > + > +static void print_help(void) > +{ > + printf("Usage: tst_res_ filename lineno [TPASS|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n"); > +} > + > +int main(int argc, char *argv[]) > +{ > + int type, i; > + > + if (argc < 5) > + goto help; > + > + if (!strcmp(argv[3], "TPASS")) > + type = TPASS; > + else if (!strcmp(argv[3], "TFAIL")) > + type = TFAIL; > + else if (!strcmp(argv[3], "TCONF")) > + type = TCONF; > + else if (!strcmp(argv[3], "TINFO")) > + type = TINFO; > + else if (!strcmp(argv[3], "TDEBUG")) > + type = TDEBUG; > + else > + goto help; > + > + size_t len = 0; > + > + for (i = 4; i < argc; i++) > + len += strlen(argv[i]) + 1; > + > + char *msg = SAFE_MALLOC(len); > + char *msgp = msg; > + > + for (i = 4; i < argc; i++) { > + msgp = strcpy(msgp, argv[i]); > + msgp += strlen(argv[i]); > + *(msgp++) = ' '; > + } > + > + *(msgp - 1) = 0; > + > + tst_reinit(); > + > + tst_res_(argv[1], atoi(argv[2]), type, msg); > + > + return 0; > +help: > + print_help(); > + return 1; > +}
Hi! > The exit command above will ignore any other result flags already set > during the test. You could modify tst_res_ to return the correct exit > code and then call it like this: It does not work like this. The tst_res helper modifies the shm memory that the test library uses, so all test results are propagated once the tst_res helper returns. Only the TBROK flag is not propagated via the shared memory in the new test library which I consider a mistake when I designed the new test library and we should move it to the shared memory...
On 24. 07. 24 11:47, Cyril Hrubis wrote: > Hi! >> The exit command above will ignore any other result flags already set >> during the test. You could modify tst_res_ to return the correct exit >> code and then call it like this: > > It does not work like this. The tst_res helper modifies the shm memory > that the test library uses, so all test results are propagated once the > tst_res helper returns. > > Only the TBROK flag is not propagated via the shared memory in the new > test library which I consider a mistake when I designed the new test > library and we should move it to the shared memory... That still doesn't solve the incorrect return value from the script if it's intentionally possible to run it as a stand-alone test, without any parent process written in C.
Hi! > That still doesn't solve the incorrect return value from the script if > it's intentionally possible to run it as a stand-alone test, without any > parent process written in C. I do not think that it will make sense or be possible to use these scripts without the C library. The whole point of this patchset is to use as much of the C library as possible so that we do not have to reimplement it in shell. Which would mean that most of the functionality needed to run these shell tests will not we available without the C library. Maybe these script shouldn't have a shebang and shouldn't have executable flag so that people are not confused about this. The test library can run them with /bin/sh $filename instead.
Hi! > $PWD is not the correct path. It should be set as follows: > > testdir=$(dirname $0) > export PATH=$PATH:$testdir:$testdir/tests/ This does not work for tests that have .needs_tmpdir because $(dirname $0) is '.' I suppose that we need realpath as well: testdir=$(realpath $(dirname $0))
diff --git a/include/tst_test.h b/include/tst_test.h index 517c8d032..fde8a1414 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -331,6 +331,8 @@ struct tst_fs { * @child_needs_reinit: Has to be set if the test needs to call tst_reinit() * from a process started by exec(). * + * @runs_shell: Implies child_needs_reinit and forks_child at the moment. + * * @needs_devfs: If set the devfs is mounted at tst_test.mntpoint. This is * needed for tests that need to create device files since tmpfs * at /tmp is usually mounted with 'nodev' option. @@ -514,6 +516,7 @@ struct tst_fs { unsigned int mount_device:1; unsigned int needs_rofs:1; unsigned int child_needs_reinit:1; + unsigned int runs_shell:1; unsigned int needs_devfs:1; unsigned int restore_wallclock:1; @@ -522,6 +525,8 @@ struct tst_fs { unsigned int skip_in_lockdown:1; unsigned int skip_in_secureboot:1; unsigned int skip_in_compat:1; + + int needs_abi_bits; unsigned int needs_hugetlbfs:1; @@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self) */ void tst_reinit(void); +/** + * tst_run_shell() - Prepare the environment and execute a shell script. + * + * @script_name: A filename of the script. + * @params: A NULL terminated array of shell script parameters, pass NULL if + * none are needed. This what is passed starting from argv[1]. + * + * The shell script is executed with LTP_IPC_PATH in environment so that the + * binary helpers such as tst_res_ or tst_checkpoint work properly when executed + * from the script. This also means that the tst_test.runs_shell flag needs to + * be set. + * + * The shell script itself has to source the tst_env.sh shell script at the + * start and after that it's free to use tst_res in the same way C code would + * use. + * + * Example shell script that reports success:: + * + * #!/bin/sh + * . tst_env.sh + * + * tst_res TPASS "Example test works" + * + * The call returns a pid in a case that you want to examine the return value + * of the script yourself. If you do not need to check the return value + * yourself you can use tst_reap_children() to wait for the completion. Or let + * the test library collect the child automatically, just be wary that the + * script and the test both runs concurently at the same time in this case. + * + * Return: A pid of the shell process. + */ +int tst_run_shell(char *script_name, char *const params[]); + unsigned int tst_multiply_timeout(unsigned int timeout); /* diff --git a/lib/tst_test.c b/lib/tst_test.c index e5bc5bf4d..fa0907353 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -173,6 +173,52 @@ void tst_reinit(void) SAFE_CLOSE(fd); } +extern char **environ; + +static unsigned int params_array_len(char *const array[]) +{ + unsigned int ret = 0; + + if (!array) + return 0; + + while (*(array++)) + ret++; + + return ret; +} + +int tst_run_shell(char *script_name, char *const params[]) +{ + int pid; + unsigned int i, params_len = params_array_len(params); + char *argv[params_len + 2] = {}; + + if (!tst_test->runs_shell) + tst_brk(TBROK, "runs_shell flag must be set!"); + + argv[0] = script_name; + + if (params) { + for (i = 0; i < params_len; i++) + argv[i+1] = params[i]; + } + + pid = SAFE_FORK(); + if (pid) + return pid; + + char *script_name_env = NULL; + SAFE_ASPRINTF(&script_name_env, "LTP_SCRIPT_NAME=%s", script_name); + putenv(script_name_env); + + execvpe(script_name, argv, environ); + + tst_brk(TBROK | TERRNO, "execv(%s, ...) failed!", script_name); + + return -1; +} + static void update_results(int ttype) { if (!results) @@ -1224,6 +1270,11 @@ static void do_setup(int argc, char *argv[]) tdebug = 1; } + if (tst_test->runs_shell) { + tst_test->child_needs_reinit = 1; + tst_test->forks_child = 1; + } + if (tst_test->needs_kconfigs && tst_kconfig_check(tst_test->needs_kconfigs)) tst_brk(TCONF, "Aborting due to unsuitable kernel config, see above!"); diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore index e8afd06f3..d0dacf62a 100644 --- a/testcases/lib/.gitignore +++ b/testcases/lib/.gitignore @@ -23,3 +23,4 @@ /tst_sleep /tst_supported_fs /tst_timeout_kill +/tst_res_ diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile index 990b46089..928d76d62 100644 --- a/testcases/lib/Makefile +++ b/testcases/lib/Makefile @@ -13,6 +13,6 @@ MAKE_TARGETS := tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\ tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\ tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\ tst_check_kconfigs tst_cgctl tst_fsfreeze tst_ns_create tst_ns_exec\ - tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled + tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_ -include $(top_srcdir)/include/mk/generic_leaf_target.mk +include $(top_srcdir)/include/mk/generic_trunk_target.mk diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh new file mode 100755 index 000000000..88607b146 --- /dev/null +++ b/testcases/lib/run_tests.sh @@ -0,0 +1,10 @@ +#!/bin/sh + +export PATH=$PATH:$PWD:$PWD/tests/ + +./tests/shell_test01 +./tests/shell_test02 +./tests/shell_test03 +./tests/shell_test04 +./tests/shell_test05 +./tests/shell_test06 diff --git a/testcases/lib/tests/.gitignore b/testcases/lib/tests/.gitignore new file mode 100644 index 000000000..da967c4d6 --- /dev/null +++ b/testcases/lib/tests/.gitignore @@ -0,0 +1,6 @@ +shell_test01 +shell_test02 +shell_test03 +shell_test04 +shell_test05 +shell_test06 diff --git a/testcases/lib/tests/Makefile b/testcases/lib/tests/Makefile new file mode 100644 index 000000000..5a5cf5310 --- /dev/null +++ b/testcases/lib/tests/Makefile @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (C) 2009, Cisco Systems Inc. +# Ngie Cooper, August 2009 + +top_srcdir ?= ../../.. + +include $(top_srcdir)/include/mk/testcases.mk + +INSTALL_TARGETS= + +include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh new file mode 100644 index 000000000..c3b3cf5fd --- /dev/null +++ b/testcases/lib/tests/shell_loader.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env tst_run_shell + +. tst_env.sh + +tst_res TPASS "Shell loader works fine!" diff --git a/testcases/lib/tests/shell_test01.c b/testcases/lib/tests/shell_test01.c new file mode 100644 index 000000000..18f834138 --- /dev/null +++ b/testcases/lib/tests/shell_test01.c @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Shell test example. + */ + +#include "tst_test.h" + +static void run_test(void) +{ + tst_run_shell("shell_test_pass.sh", NULL); + tst_res(TINFO, "C test exits now"); +} + +static struct tst_test test = { + .runs_shell = 1, + .test_all = run_test, +}; diff --git a/testcases/lib/tests/shell_test02.c b/testcases/lib/tests/shell_test02.c new file mode 100644 index 000000000..3b0534370 --- /dev/null +++ b/testcases/lib/tests/shell_test02.c @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Shell test example. + */ + +#include "tst_test.h" + +static void run_test(void) +{ + tst_run_shell("shell_test_pass.sh", NULL); + tst_reap_children(); + tst_res(TINFO, "Shell test has finished at this point!"); +} + +static struct tst_test test = { + .runs_shell = 1, + .test_all = run_test, +}; diff --git a/testcases/lib/tests/shell_test03.c b/testcases/lib/tests/shell_test03.c new file mode 100644 index 000000000..c11a09e4e --- /dev/null +++ b/testcases/lib/tests/shell_test03.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Shell test example. + */ + +#include <sys/wait.h> +#include "tst_test.h" + +static void run_test(void) +{ + int pid, status; + + pid = tst_run_shell("shell_test_pass.sh", NULL); + + tst_res(TINFO, "Waiting for the pid %i", pid); + + waitpid(pid, &status, 0); + + tst_res(TINFO, "Shell test has %s", tst_strstatus(status)); +} + +static struct tst_test test = { + .runs_shell = 1, + .test_all = run_test, +}; diff --git a/testcases/lib/tests/shell_test04.c b/testcases/lib/tests/shell_test04.c new file mode 100644 index 000000000..fd1fd122c --- /dev/null +++ b/testcases/lib/tests/shell_test04.c @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Shell test example. + */ + +#include "tst_test.h" + +static void run_test(void) +{ + char *const params[] = {"param1", "param2", NULL}; + + tst_run_shell("shell_test_check_argv.sh", params); +} + +static struct tst_test test = { + .runs_shell = 1, + .test_all = run_test, +}; diff --git a/testcases/lib/tests/shell_test05.c b/testcases/lib/tests/shell_test05.c new file mode 100644 index 000000000..2141d4790 --- /dev/null +++ b/testcases/lib/tests/shell_test05.c @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Shell test example. + */ + +#include "tst_test.h" + +static void run_test(void) +{ + int pid; + + pid = tst_run_shell("shell_test_checkpoint.sh", NULL); + + tst_res(TINFO, "Waiting for shell to sleep on checkpoint!"); + + TST_PROCESS_STATE_WAIT(pid, 'S', 10000); + + tst_res(TINFO, "Waking shell child!"); + + TST_CHECKPOINT_WAKE(0); +} + +static struct tst_test test = { + .runs_shell = 1, + .needs_checkpoints = 1, + .test_all = run_test, +}; diff --git a/testcases/lib/tests/shell_test06.c b/testcases/lib/tests/shell_test06.c new file mode 100644 index 000000000..25abc1e7b --- /dev/null +++ b/testcases/lib/tests/shell_test06.c @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Shell test example. + */ + +#include "tst_test.h" + +static void run_test(void) +{ + tst_run_shell("shell_test_brk.sh", NULL); +} + +static struct tst_test test = { + .runs_shell = 1, + .test_all = run_test, +}; diff --git a/testcases/lib/tests/shell_test_brk.sh b/testcases/lib/tests/shell_test_brk.sh new file mode 100755 index 000000000..f266dc3fe --- /dev/null +++ b/testcases/lib/tests/shell_test_brk.sh @@ -0,0 +1,6 @@ +#!/bin/sh + +. tst_env.sh + +tst_brk TCONF "This exits test and the next message should not be reached" +tst_res TFAIL "If you see this the test failed" diff --git a/testcases/lib/tests/shell_test_check_argv.sh b/testcases/lib/tests/shell_test_check_argv.sh new file mode 100755 index 000000000..756189ee7 --- /dev/null +++ b/testcases/lib/tests/shell_test_check_argv.sh @@ -0,0 +1,25 @@ +#!/bin/sh + +. tst_env.sh + +echo "$@" + +tst_res TINFO "argv = $@" + +if [ $# -ne 2 ]; then + tst_res TFAIL "Wrong number of parameters got $# expected 2" +else + tst_res TPASS "Got 2 parameters" +fi + +if [ "$1" != "param1" ]; then + tst_res TFAIL "First parameter is $1 expected param1" +else + tst_res TPASS "First parameter is $1" +fi + +if [ "$2" != "param2" ]; then + tst_res TFAIL "Second parameter is $2 expected param2" +else + tst_res TPASS "Second parameter is $2" +fi diff --git a/testcases/lib/tests/shell_test_checkpoint.sh b/testcases/lib/tests/shell_test_checkpoint.sh new file mode 100755 index 000000000..0ceb7cf66 --- /dev/null +++ b/testcases/lib/tests/shell_test_checkpoint.sh @@ -0,0 +1,7 @@ +#!/bin/sh + +. tst_env.sh + +tst_res TINFO "Waiting for a checkpoint 0" +tst_checkpoint wait 10000 0 +tst_res TPASS "Continuing after checkpoint" diff --git a/testcases/lib/tests/shell_test_pass.sh b/testcases/lib/tests/shell_test_pass.sh new file mode 100755 index 000000000..fd0684eb2 --- /dev/null +++ b/testcases/lib/tests/shell_test_pass.sh @@ -0,0 +1,6 @@ +#!/bin/sh + +. tst_env.sh + +tst_res TPASS "This is called from the shell script!" +tst_sleep 100ms diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh new file mode 100644 index 000000000..c8f3c2160 --- /dev/null +++ b/testcases/lib/tst_env.sh @@ -0,0 +1,16 @@ +#!/bin/sh + +tst_script_name=$(basename $0) + +tst_brk_() +{ + tst_res_ $@ + + case "$3" in + "TBROK") exit 2;; + *) exit 0;; + esac +} + +alias tst_res="tst_res_ $tst_script_name \$LINENO" +alias tst_brk="tst_brk_ $tst_script_name \$LINENO" diff --git a/testcases/lib/tst_res_.c b/testcases/lib/tst_res_.c new file mode 100644 index 000000000..cc3fa53d3 --- /dev/null +++ b/testcases/lib/tst_res_.c @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz> + */ + +#define TST_NO_DEFAULT_MAIN +#include "tst_test.h" + +static void print_help(void) +{ + printf("Usage: tst_res_ filename lineno [TPASS|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n"); +} + +int main(int argc, char *argv[]) +{ + int type, i; + + if (argc < 5) + goto help; + + if (!strcmp(argv[3], "TPASS")) + type = TPASS; + else if (!strcmp(argv[3], "TFAIL")) + type = TFAIL; + else if (!strcmp(argv[3], "TCONF")) + type = TCONF; + else if (!strcmp(argv[3], "TINFO")) + type = TINFO; + else if (!strcmp(argv[3], "TDEBUG")) + type = TDEBUG; + else + goto help; + + size_t len = 0; + + for (i = 4; i < argc; i++) + len += strlen(argv[i]) + 1; + + char *msg = SAFE_MALLOC(len); + char *msgp = msg; + + for (i = 4; i < argc; i++) { + msgp = strcpy(msgp, argv[i]); + msgp += strlen(argv[i]); + *(msgp++) = ' '; + } + + *(msgp - 1) = 0; + + tst_reinit(); + + tst_res_(argv[1], atoi(argv[2]), type, msg); + + return 0; +help: + print_help(); + return 1; +}
This is a proof of a concept of a seamless C and shell integration. The idea is that with this you can mix shell and C code as much as as you wish to get the best of the two worlds. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- include/tst_test.h | 38 +++++++++++++ lib/tst_test.c | 51 +++++++++++++++++ testcases/lib/.gitignore | 1 + testcases/lib/Makefile | 4 +- testcases/lib/run_tests.sh | 10 ++++ testcases/lib/tests/.gitignore | 6 ++ testcases/lib/tests/Makefile | 11 ++++ testcases/lib/tests/shell_loader.sh | 5 ++ testcases/lib/tests/shell_test01.c | 17 ++++++ testcases/lib/tests/shell_test02.c | 18 ++++++ testcases/lib/tests/shell_test03.c | 25 +++++++++ testcases/lib/tests/shell_test04.c | 18 ++++++ testcases/lib/tests/shell_test05.c | 27 +++++++++ testcases/lib/tests/shell_test06.c | 16 ++++++ testcases/lib/tests/shell_test_brk.sh | 6 ++ testcases/lib/tests/shell_test_check_argv.sh | 25 +++++++++ testcases/lib/tests/shell_test_checkpoint.sh | 7 +++ testcases/lib/tests/shell_test_pass.sh | 6 ++ testcases/lib/tst_env.sh | 16 ++++++ testcases/lib/tst_res_.c | 58 ++++++++++++++++++++ 20 files changed, 363 insertions(+), 2 deletions(-) create mode 100755 testcases/lib/run_tests.sh create mode 100644 testcases/lib/tests/.gitignore create mode 100644 testcases/lib/tests/Makefile create mode 100644 testcases/lib/tests/shell_loader.sh create mode 100644 testcases/lib/tests/shell_test01.c create mode 100644 testcases/lib/tests/shell_test02.c create mode 100644 testcases/lib/tests/shell_test03.c create mode 100644 testcases/lib/tests/shell_test04.c create mode 100644 testcases/lib/tests/shell_test05.c create mode 100644 testcases/lib/tests/shell_test06.c create mode 100755 testcases/lib/tests/shell_test_brk.sh create mode 100755 testcases/lib/tests/shell_test_check_argv.sh create mode 100755 testcases/lib/tests/shell_test_checkpoint.sh create mode 100755 testcases/lib/tests/shell_test_pass.sh create mode 100644 testcases/lib/tst_env.sh create mode 100644 testcases/lib/tst_res_.c