diff mbox

fix posix/tst-spawn test

Message ID 59CBB879.5060707@arm.com
State New, archived
Headers show

Commit Message

Szabolcs Nagy Sept. 27, 2017, 2:40 p.m. UTC 
On 27/09/17 14:00, Florian Weimer wrote:
> On 09/27/2017 02:56 PM, Szabolcs Nagy wrote:
>> +  /* Wait for the child.  */
>> +  if (waitpid (pid, &status, 0) != pid)
>> +    error (EXIT_FAILURE, errno, "wrong child");
> 
> You could use
> 
>   TEST_VERIFY (xwaitpid (pid, &status, 0) != pid));
> 
> instead.
> 
> In fact, all the error calls are invalid in tests because they write to standard error.
> 
> So perhaps use
> 
>   TEST_VERIFY (WIFEXITED (status));
>   TEST_VERIFY (!WIFSIGNALED (status));
>   TEST_VERIFY (WEXITSTATUS (status) == 0);
> 
> The error messages did not contain the status bits anyway, so this is not a regression as far as diagnostics
> are concerned.

i didn't want to fix the entire test to use the new
conventions, so i only changed these lines.


The test spawns two children but only waited for one.
The fix avoids printing to stderr.

2017-09-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	* posix/tst-spawn.c (do_test): Wait for both children.

Comments

Florian Weimer Sept. 27, 2017, 2:45 p.m. UTC | #1 
On 09/27/2017 04:40 PM, Szabolcs Nagy wrote:
> The test spawns two children but only waited for one.
> The fix avoids printing to stderr.
> 
> 2017-09-27  Szabolcs Nagy<szabolcs.nagy@arm.com>
> 
> 	* posix/tst-spawn.c (do_test): Wait for both children.

Sorry, one more thing: I think you should not interleave the spawns and 
waits.  I think you can do a -1 wait after the PID wait, and the test 
will still pass.  This should be closer to the original test.

Thanks,
Florian
diff mbox

Patch

diff --git a/posix/tst-spawn.c b/posix/tst-spawn.c
index 08d92bd7a7..851978f224 100644
--- a/posix/tst-spawn.c
+++ b/posix/tst-spawn.c
@@ -23,9 +23,10 @@ 
 #include <spawn.h>
 #include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
 #include <wait.h>
 #include <sys/param.h>
+#include <support/check.h>
+#include <support/xunistd.h>
 
 
 /* Nonzero if the program gets called via `exec'.  */
@@ -240,22 +241,26 @@  do_test (int argc, char *argv[])
    if (posix_spawn (&pid, argv[1], &actions, NULL, spargv, environ) != 0)
      error (EXIT_FAILURE, errno, "posix_spawn");
 
+  /* Wait for the child.  */
+  TEST_VERIFY (xwaitpid (pid, &status, 0) == pid);
+  TEST_VERIFY (WIFEXITED (status));
+  TEST_VERIFY (!WIFSIGNALED (status));
+  TEST_VERIFY (WEXITSTATUS (status) == 0);
+
    /* Same test but with a NULL pid argument.  */
    if (posix_spawn (NULL, argv[1], &actions, NULL, spargv, environ) != 0)
      error (EXIT_FAILURE, errno, "posix_spawn");
 
+  /* Wait for the child.  */
+  xwaitpid (-1, &status, 0);
+  TEST_VERIFY (WIFEXITED (status));
+  TEST_VERIFY (!WIFSIGNALED (status));
+  TEST_VERIFY (WEXITSTATUS (status) == 0);
+
    /* Cleanup.  */
    if (posix_spawn_file_actions_destroy (&actions) != 0)
      error (EXIT_FAILURE, errno, "posix_spawn_file_actions_destroy");
    free (name3_copy);
 
-  /* Wait for the child.  */
-  if (waitpid (pid, &status, 0) != pid)
-    error (EXIT_FAILURE, errno, "wrong child");
-
-  if (WTERMSIG (status) != 0)
-    error (EXIT_FAILURE, 0, "Child terminated incorrectly");
-  status = WEXITSTATUS (status);
-
-  return status;
+  return 0;
 }