diff mbox

fix posix/tst-spawn test

Message ID 59CBBB68.60205@arm.com
State Committed
Commit 5ba41de9ed2cf6cf39a9cd7a7541bc71fc447d52
Headers show

Commit Message

Szabolcs Nagy Sept. 27, 2017, 2:53 p.m. UTC 
On 27/09/17 15:45, Florian Weimer wrote:
> 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.
> 

why does that matter?
(attached the updated patch)

Comments

Florian Weimer Oct. 12, 2017, 1:32 p.m. UTC | #1 
On 09/27/2017 04:53 PM, Szabolcs Nagy wrote:
>> 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.
>>
> why does that matter?

It checks that you can actually launch multiple child processes.

> diff --git a/posix/tst-spawn.c b/posix/tst-spawn.c
> index 08d92bd7a7..4e5e76351c 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'.  */
> @@ -249,13 +250,16 @@ do_test (int argc, char *argv[])
>        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");
> +  /* Wait for the children.  */
> +  TEST_VERIFY (xwaitpid (pid, &status, 0) == pid);
> +  TEST_VERIFY (WIFEXITED (status));
> +  TEST_VERIFY (!WIFSIGNALED (status));
> +  TEST_VERIFY (WEXITSTATUS (status) == 0);
>   
> -  if (WTERMSIG (status) != 0)
> -    error (EXIT_FAILURE, 0, "Child terminated incorrectly");
> -  status = WEXITSTATUS (status);
> +  xwaitpid (-1, &status, 0);
> +  TEST_VERIFY (WIFEXITED (status));
> +  TEST_VERIFY (!WIFSIGNALED (status));
> +  TEST_VERIFY (WEXITSTATUS (status) == 0);
>   
> -  return status;
> +  return 0;
>   }

Looks good to me, thanks.

Florian
diff mbox

Patch

diff --git a/posix/tst-spawn.c b/posix/tst-spawn.c
index 08d92bd7a7..4e5e76351c 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'.  */
@@ -249,13 +250,16 @@  do_test (int argc, char *argv[])
      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");
+  /* Wait for the children.  */
+  TEST_VERIFY (xwaitpid (pid, &status, 0) == pid);
+  TEST_VERIFY (WIFEXITED (status));
+  TEST_VERIFY (!WIFSIGNALED (status));
+  TEST_VERIFY (WEXITSTATUS (status) == 0);
 
-  if (WTERMSIG (status) != 0)
-    error (EXIT_FAILURE, 0, "Child terminated incorrectly");
-  status = WEXITSTATUS (status);
+  xwaitpid (-1, &status, 0);
+  TEST_VERIFY (WIFEXITED (status));
+  TEST_VERIFY (!WIFSIGNALED (status));
+  TEST_VERIFY (WEXITSTATUS (status) == 0);
 
-  return status;
+  return 0;
 }