Re: [PATCH] re-fix child reaping in wordexp

[Rich Felker <dalias@libc.org>]

On Mon, Feb 05, 2018 at 05:38:37PM +0300, Alexander Monakov wrote:
> Do not retry waitpid if the child was terminated by a signal. Do not
> examine status: since we are not passing any flags, we will not receive
> stop or continue notifications.

Looks fine.

> ---
> 
> In general retrying waitpid on EINTR is not robust in case pid reuse is
> possible, but fixing that requires changing waitpid call sites to only
> do that with signals blocked (where that's not already the case).

I don't follow this. Unless there's a bug in the kernel, this should
not be functionally different from SA_RESTART. A return with EINTR
means the child was not reaped.

Rich


> 
>  src/misc/wordexp.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/src/misc/wordexp.c b/src/misc/wordexp.c
> index db39b5b8..d123cf75 100644
> --- a/src/misc/wordexp.c
> +++ b/src/misc/wordexp.c
> @@ -14,13 +14,7 @@
>  static void reap(pid_t pid)
>  {
>  	int status;
> -	for (;;) {
> -		if (waitpid(pid, &status, 0) < 0) {
> -			if (errno != EINTR) return;
> -		} else {
> -			if (WIFEXITED(status)) return;
> -		}
> -	}
> +	while (waitpid(pid, &status, 0) < 0 && errno == EINTR);
>  }
>  
>  static char *getword(FILE *f)
> -- 
> 2.11.0