Re: [PATCH v7 4/4] posix: Add posix_spawn_file_actions_addclosefrom_np

["H.J. Lu via Libc-alpha" <libc-alpha@sourceware.org>]

On Thu, Jul 8, 2021 at 9:12 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 08/07/2021 11:34, Florian Weimer wrote:
> > Some of the new files use:
> >
> >> +   <http://www.gnu.org/licenses/>.  */
> >
> > That should be: https://
>
> Ack.
>
> >
> >> +/* Called on process re-execution.  The arguments are the expected opened
> >> +   file descriptors.  */
> >> +_Noreturn static void
> >> +handle_restart (int argc, char *argv[])
> >> +{
> >> +  size_t nfds = argc > 1 ? argc - 1 : 0;
> >> +  struct fd_t
> >> +  {
> >> +    int fd;
> >> +    _Bool found;
> >> +  } *fds = xmalloc (sizeof (struct fd_t) * nfds);
> >> +  for (int i = 0; i < nfds; i++)
> >> +    {
> >> +      char *endptr;
> >> +      long unsigned int fd = strtoul (argv[i+1], &endptr, 10);
> >> +      if (*endptr != '\0' || fd > INT_MAX)
> >> +        FAIL_EXIT1 ("readdir: invalid file descriptor value: %s", argv[i]);
> >
> > Should be:
> >
> >   FAIL_EXIT1 ("argv[%d]: invalid file descriptor value: %s", i, argv[i]);
>
> Ack.
>
> >
> >> @@ -344,7 +346,9 @@ __spawnix (pid_t * pid, const char *file,
> >>    /* We need at least a few pages in case the compiler's stack checking is
> >>       enabled.  In some configs, it is known to use at least 24KiB.  We use
> >>       32KiB to be "safe" from anything the compiler might do.  Besides, the
> >> -     extra pages won't actually be allocated unless they get used.  */
> >> +     extra pages won't actually be allocated unless they get used.
> >> +     It also acts the slack for spawn_closefrom (including MIPS64 getdents64
> >> +     where it might use about 1k extra stack space.  */
> >
> > Missing closing parenthesis.
> >
> > Rest looks okay to me, thanks.  No need to post a new verison, I think.
>
> Thanks, I will respin another round of tests and commit.

On x86-64, I saw:

FAIL: posix/tst-spawn5

https://sourceware.org/bugzilla/show_bug.cgi?id=28067

Has anyone else seen it?

-- 
H.J.