diff mbox series

[4/4] stdlib: Move tst-system to tests-container

Message ID 20200324185043.2073568-4-adhemerval.zanella@linaro.org
State New
Headers show
Series [1/4] support/shell-container.c: Return 127 if execve fails | expand

Commit Message

Michael Kerrisk \(man-pages\) via Libc-alpha March 24, 2020, 6:50 p.m. UTC
Fix some issues with different shell and error messages.

Checked on x86_64-linux-gnu.
---
 stdlib/Makefile     |  3 ++-
 stdlib/tst-system.c | 10 ++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

Comments

Michael Kerrisk \(man-pages\) via Libc-alpha March 25, 2020, 12:03 p.m. UTC | #1
Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> writes:

> Fix some issues with different shell and error messages.
[...]
> +    char *returnerr = xasprintf ("%s: execing %s failed: "
> +				 "No such file or directory",
>  				 basename(_PATH_BSHELL), cmd);
>      TEST_COMPARE_STRING (result.err.buffer, returnerr);

Yesterday's cherry-pick of this new test to the 2.31 branch breaks the
test suite for me with bash 5.0.11 as /bin/sh. Here's tst-system.out,
trimmed a bit:

| tst-system.c:93: error: string comparison failed
|   left string: 279 bytes
|   right string: 274 bytes
|   left (evaluated from result.err.buffer):
|       "sh: aaa...aaaaaa: command not found\n"
|   right (evaluated from returnerr):
|       "sh: 1: aaa...aaaaaa: not found\n"
| tst-system.c:111: error: string comparison failed
|   left string: 280 bytes
|   right string: 284 bytes
|   left (evaluated from result.err.buffer):
|       "sh: aaa...aaaaaaa: command not found\n"
|   right (evaluated from returnerr):
|       "sh: 1: aaa...aaaaaaa: File name too long\n"
| error: 2 test failures

I don't think that trying to match against the shell error message here
is a good idea, since it'll depend on what /bin/sh the user has:

$ bash -c blargh
bash: blargh: command not found
$ dash -c blargh
dash: 1: blargh: not found
$ mksh -c blargh
mksh: blargh: inaccessible or not found

Thanks,
Michael Kerrisk \(man-pages\) via Libc-alpha March 25, 2020, 12:11 p.m. UTC | #2
On 25/03/2020 09:03, Adam Sampson via Libc-alpha wrote:
> Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> writes:
> 
>> Fix some issues with different shell and error messages.
> [...]
>> +    char *returnerr = xasprintf ("%s: execing %s failed: "
>> +				 "No such file or directory",
>>  				 basename(_PATH_BSHELL), cmd);
>>      TEST_COMPARE_STRING (result.err.buffer, returnerr);
> 
> Yesterday's cherry-pick of this new test to the 2.31 branch breaks the
> test suite for me with bash 5.0.11 as /bin/sh. Here's tst-system.out,
> trimmed a bit:
> 
> | tst-system.c:93: error: string comparison failed
> |   left string: 279 bytes
> |   right string: 274 bytes
> |   left (evaluated from result.err.buffer):
> |       "sh: aaa...aaaaaa: command not found\n"
> |   right (evaluated from returnerr):
> |       "sh: 1: aaa...aaaaaa: not found\n"
> | tst-system.c:111: error: string comparison failed
> |   left string: 280 bytes
> |   right string: 284 bytes
> |   left (evaluated from result.err.buffer):
> |       "sh: aaa...aaaaaaa: command not found\n"
> |   right (evaluated from returnerr):
> |       "sh: 1: aaa...aaaaaaa: File name too long\n"
> | error: 2 test failures
> 
> I don't think that trying to match against the shell error message here
> is a good idea, since it'll depend on what /bin/sh the user has:
> 
> $ bash -c blargh
> bash: blargh: command not found
> $ dash -c blargh
> dash: 1: blargh: not found
> $ mksh -c blargh
> mksh: blargh: inaccessible or not found
> 
> Thanks,
> '

Yes, the patchset it exactly to remove this dependency and use
the libsupport shell-container.c instead.
diff mbox series

Patch

diff --git a/stdlib/Makefile b/stdlib/Makefile
index 45214b59e4..4615f6dfe7 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -70,7 +70,7 @@  tests		:= tst-strtol tst-strtod testmb testrand testsort testdiv   \
 		   test-canon test-canon2 tst-strtoll tst-environ	    \
 		   tst-xpg-basename tst-random tst-random2 tst-bsearch	    \
 		   tst-limits tst-rand48 bug-strtod tst-setcontext	    \
-		   tst-setcontext2 test-a64l tst-qsort tst-system testmb2   \
+		   tst-setcontext2 test-a64l tst-qsort testmb2              \
 		   bug-strtod2 tst-atof1 tst-atof2 tst-strtod2		    \
 		   tst-rand48-2 tst-makecontext tst-strtod5		    \
 		   tst-qsort2 tst-makecontext2 tst-strtod6 tst-unsetenv1    \
@@ -92,6 +92,7 @@  tests		:= tst-strtol tst-strtod testmb testrand testsort testdiv   \
 tests-internal	:= tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \
 		   tst-tls-atexit tst-tls-atexit-nodelete
 tests-static	:= tst-secure-getenv
+tests-container := tst-system
 
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-empty-env
diff --git a/stdlib/tst-system.c b/stdlib/tst-system.c
index c8c1811349..eddea33f4c 100644
--- a/stdlib/tst-system.c
+++ b/stdlib/tst-system.c
@@ -88,7 +88,8 @@  do_test (void)
 					 });
     support_capture_subprocess_check (&result, "system", 0, sc_allow_stderr);
 
-    char *returnerr = xasprintf ("%s: 1: %s: not found\n",
+    char *returnerr = xasprintf ("%s: execing %s failed: "
+				 "No such file or directory",
 				 basename(_PATH_BSHELL), cmd);
     TEST_COMPARE_STRING (result.err.buffer, returnerr);
     free (returnerr);
@@ -106,7 +107,8 @@  do_test (void)
 					 });
     support_capture_subprocess_check (&result, "system", 0, sc_allow_stderr);
 
-    char *returnerr = xasprintf ("%s: 1: %s: File name too long\n",
+    char *returnerr = xasprintf ("%s: execing %s failed: "
+				 "File name too long",
 				 basename(_PATH_BSHELL), cmd);
     TEST_COMPARE_STRING (result.err.buffer, returnerr);
     free (returnerr);
@@ -116,7 +118,7 @@  do_test (void)
     struct support_capture_subprocess result;
     result = support_capture_subprocess (call_system,
 					 &(struct args) {
-					   "kill -USR1 $$", 0, SIGUSR1
+					   "kill $$", 0, SIGTERM
 					 });
     support_capture_subprocess_check (&result, "system", 0, sc_allow_none);
   }
@@ -136,7 +138,7 @@  do_test (void)
     support_capture_subprocess_check (&result, "system", 0, sc_allow_none);
   }
 
-  TEST_COMPARE (system (":"), 0);
+  TEST_COMPARE (system (""), 0);
 
   return 0;
 }