Re: [hackers] [sbase][PATCH 3/6] find: Fix spawn() return value

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.net>
Date: Mon, 17 Mar 2025 12:59:56 +0100

Hi,

Quoth Evan Gates <evan.gates_AT_gmail.com>:
> On Fri, 14 Feb 2025 at 09:25 Tavian Barnes, <tavianator_AT_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
> > _AT_@ -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

Indeed, I think this change is not needed.

> > case 0:
> > execvp(*argv, argv);
> > weprintf("exec %s failed:", *argv);
> > _AT_@ -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?
>
> > }

Indeed. The WIFEXITED and WEXITSTATUS are only required in other
cases where you care about the error reason, but for success
you don't need them.

Kind regards,
                 
Received on Mon Mar 17 2025 - 12:59:56 CET