On Mon, Feb 17, 2025 at 6:30 PM Evan Gates <evan.ga...@gmail.com> wrote:
> On Fri, 14 Feb 2025 at 09:25 Tavian Barnes, <taviana...@tavianator.com> wrote:
>
> > ---
> >  find.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/find.c b/find.c
> > index ce551e5..71488da 100644
> > --- a/find.c
> > +++ b/find.c
> > @@ -244,6 +244,7 @@ spawn(char *argv[])
> >         switch((pid = fork())) {
> >         case -1:
> >                 eprintf("fork:");
> > +               return -1;
>
> eprintf calls exit(1), there's no return to do here

Ah okay, makes sense.

> >         case 0:
> >                 execvp(*argv, argv);
> >                 weprintf("exec %s failed:", *argv);
> > @@ -252,7 +253,7 @@ spawn(char *argv[])
> >
> >         /* FIXME: proper course of action for waitpid() on EINTR? */
> >         waitpid(pid, &status, 0);
> > -       return status;
> > +       return WIFEXITED(status) && WEXITSTATUS(status) == 0 ? 0 : -1;
>
> From POSIX wait() documention:
>
>         The value stored at the location pointed to by stat_loc shall
>         be 0 if and only if the status returned is from a terminated
>         child process that terminated by one of the following means:
>
>                 1. The process returned 0 from main().
>
>                 2. The process called _exit() or exit() with a status
>                 argument of 0.
>
>                 3. The process was terminated because the last thread
>                 in the process terminated.
>
> Why do extra work if we already have 0 status on success?

I didn't realize POSIX specified the encoding of a successful exit,
neat.  On further testing I can see this patch doesn't change
anything.  I'm not sure why I thought it was necessary in the first
place.  Please drop it.

> >  }
> >
> >  static int
> > --
> > 2.48.1
> >
> >
>

Reply via email to