Re: [PATCH v3][BZ 21340] add support for POSIX_SPAWN_SETSID

[Florian Weimer <fweimer at redhat dot com>]

On 04/05/2017 07:41 AM, daurnimator wrote:

> +static int
> +do_test (void)
> +{
> +  posix_spawnattr_t attrp;
> +  int res;
> +  int child_pid;
> +  int sid, child_sid;
> +  char *args[2];

Thanks for writing the test.  There are some style issues.  We prefer 
in-line declarations these days, and there should be a space before “(” 
in a function call.

Please use the new test skeleton in <support/test-driver.c>.  (Sorry, 
still need to update the wiki.)

> +  posix_spawnattr_init(&attrp);
> +  if (posix_spawnattr_setflags(&attrp, POSIX_SPAWN_SETSID))
> +    {
> +      printf("error: posix_spawnattr_setflags: %m\n");

You need to capture the return value of posix_spawnattr_setflags and 
assign it to errno prior to using %m.  You can include <support/check.h> 
and use FAIL_EXIT1.

> +  /* run the program 'true' */
> +  args[0] = (char *)"true";
> +  args[1] = NULL;
> +
> +  res = posix_spawnp(&child_pid, "true", NULL, &attrp, args, environ);
> +  posix_spawnattr_destroy(&attrp);
> +  if (res)
> +    {
> +      printf("error: posix_spawnp: %m\n");

See above, needs to set errno.

> +      return 1;
> +    }
> +
> +  /* child should have a different sid */
> +  child_sid = getsid(child_pid);

Please add error checking for the getsid result.

> +  if (child_sid == sid)
> +    {
> +      printf("error: child sid matches\n");
> +      return 1;
> +    }

I think you should run the test case twice, and whether 
POSIX_SPAWN_SETSID is applied should be controlled by a flag.  Then you 
can check if the getsid result remains unchanged or is changed, as 
appropriate.

You aren't listed in MAINTAINERS on the wiki.  Do you have a GNU 
copyright assignment on file?

Thanks,
Florian