[stgt] [PATCH] fixes in call_program (closing pipe fds, waitpid interrupted by a signal)

[FUJITA Tomonori]

On Tue, 8 Feb 2011 17:39:19 +0200
Alexander Nezhinsky <alexandern at Voltaire.COM> wrote:

> Some fixes in call_program():
> 1) if fork() fails, previously open pipe file descriptors should be closed.
> 2) the redirect script is invoked from a fork'ed process so tgt has to wait
> for its child. Currently this is done synchronously using waitpid().
> Sometimes the system call is interrupted by a signal (most probably by
> a SIGCHLD signal from the very process that is being waited upon).
> To overcome this waitpid() is called repeatedly while the error code
> is "Interrupted by a signal".
> 
> Signed-off-by: Alexander Nezhinsky <alexandern at voltaire.com>
> ---
>  usr/tgtd.c |   20 +++++++++++++++-----
>  1 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/usr/tgtd.c b/usr/tgtd.c
> index c3abca8..619ecb0 100644
> --- a/usr/tgtd.c
> +++ b/usr/tgtd.c
> @@ -285,14 +285,20 @@ int call_program(const char *cmd, void (*callback)(void *data, int result),
>  	argv[i] =  NULL;
>  
>  	ret = pipe(fds);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		eprintf("pipe create failed for %s, %m\n", cmd);
>  		return ret;
> +	}
>  
> -	eprintf("%d %d\n", fds[0], fds[1]);
> +	dprintf("%s, pipe: %d %d\n", cmd, fds[0], fds[1]);
>  
>  	pid = fork();
> -	if (pid < 0)
> +	if (pid < 0) {
> +		eprintf("fork failed for: %s, %m\n", cmd);
> +		close(fds[0]);
> +		close(fds[1]);
>  		return pid;
> +	}
>  
>  	if (!pid) {
>  		close(1);
> @@ -302,10 +308,14 @@ int call_program(const char *cmd, void (*callback)(void *data, int result),
>  		exit(-1);
>  	} else {
>  		close(fds[1]);
> -		waitpid(pid, &i, 0);
> +		do {
> +			ret = waitpid(pid, &i, 0);
> +			if (ret < 0 && errno != EINTR)
> +				eprintf("waitpid failed for: %s, %m\n", cmd);

Do we need 'break' here? If we hit an error here, we need to give up
and free the allocated resource?


> +		} while (ret < 0 && errno == EINTR);
>  		ret = read(fds[0], output, op_len);
>  		if (ret < 0)
> -			eprintf("failed to get the output from <%s>.", cmd);
> +			eprintf("failed to get the output from: %s\n", cmd);
>  
>  		if (callback)
>  			callback(data, WEXITSTATUS(i));
> --
> 1.6.5.5
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html