ACK: [PATCH 4/6] pipeio: Add fwts_pipe_open_rw

[Alex Hung]

On 2016-04-22 01:41 PM, Jeremy Kerr wrote:
> This change introduces a function to open a process pipe that supports
> both reading and writing (ie, we open pipes for both stdin and stdout of
> the child process).
>
> We do this by extending and renaming the existing fwts_pipe_open_ro,
> then use the new function to implement fwts_pipe_open_ro.
>
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
> ---
>   src/lib/include/fwts_pipeio.h |  2 ++
>   src/lib/src/fwts_pipeio.c     | 74 ++++++++++++++++++++++++++++++++++---------
>   2 files changed, 61 insertions(+), 15 deletions(-)
>
> diff --git a/src/lib/include/fwts_pipeio.h b/src/lib/include/fwts_pipeio.h
> index e27d90b..e2d498a 100644
> --- a/src/lib/include/fwts_pipeio.h
> +++ b/src/lib/include/fwts_pipeio.h
> @@ -31,6 +31,8 @@
>   #define FWTS_EXEC_ERROR		(127)
>
>   int   fwts_pipe_open_ro(const char *command, pid_t *childpid, int *fd);
> +int   fwts_pipe_open_rw(const char *command, pid_t *childpid, int *in_fd,
> +		int *out_fd);
>   int   fwts_pipe_read(const int fd, char **out_buf, ssize_t *out_len);
>   int   fwts_pipe_close(const int fd, const pid_t pid);
>   int   fwts_pipe_exec(const char *command, fwts_list **list, int *status);
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index 64fc91d..bb51515 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -24,6 +24,7 @@
>
>   #include <signal.h>
>   #include <errno.h>
> +#include <fcntl.h>
>   #include <unistd.h>
>   #include <stdio.h>
>   #include <stdlib.h>
> @@ -40,48 +41,91 @@
>   #include "fwts.h"
>
>   /*
> - *  fwts_pipe_open_ro()
> + *  fwts_pipe_open_rw()
>    *	execl a command, return pid in *childpid and a pipe connected
> - *	to stdout in *fd. Return value < 0 indicates error.
> + *	to stdout in *in_fd, and stdout in *out_fd. Return value < 0
> + *	indicates error.
>    */
> -int fwts_pipe_open_ro(const char *command, pid_t *childpid, int *fd)
> +int fwts_pipe_open_rw(const char *command, pid_t *childpid,
> +		int *in_fd, int *out_fd)
>   {
> -	int pipefds[2];
> +	int out_pipefds[2];
> +	int in_pipefds[2];
>   	pid_t pid;
>   	FILE *fp;
>
> -	if (pipe(pipefds) < 0)
> -		return -1;
> +	if (in_fd != NULL) {
> +		if (pipe(in_pipefds) < 0)
> +			return -1;
> +	} else {
> +		in_pipefds[0] = open("/dev/null", O_RDONLY);
> +		in_pipefds[1] = -1;
> +	}
> +
> +	if (out_fd != NULL) {
> +		if (pipe(out_pipefds) < 0)
> +			goto err_close_in;
> +	} else {
> +		out_pipefds[0] = -1;
> +		out_pipefds[1] = open("/dev/null", O_WRONLY);
> +	}
>
>   	pid = fork();
>   	switch (pid) {
>   	case -1:
>   		/* Ooops */
> -		close(pipefds[0]);
> -		close(pipefds[1]);
> -		return -1;
> +		goto err_close_out;
>   	case 0:
>   		/* Child */
>   		if ((fp = freopen("/dev/null", "w", stderr)) == NULL) {
>   			fprintf(stderr, "Cannot redirect stderr\n");
>   		}
> -		if (pipefds[0] != STDOUT_FILENO) {
> -			dup2(pipefds[1], STDOUT_FILENO);
> -			close(pipefds[1]);
> +		if (out_pipefds[0] != STDOUT_FILENO) {
> +			dup2(out_pipefds[1], STDOUT_FILENO);
> +			close(out_pipefds[1]);
>   		}
> -		close(pipefds[0]);
> +		if (in_pipefds[1] != STDIN_FILENO) {
> +			dup2(in_pipefds[0], STDIN_FILENO);
> +			close(in_pipefds[0]);
> +		}
> +
> +		close(out_pipefds[0]);
> +		close(in_pipefds[1]);
>   		execl(_PATH_BSHELL, "sh", "-c", command, NULL);
>   		if (fp)
>   			fclose(fp);
>   		_exit(FWTS_EXEC_ERROR);
>   	default:
>   		/* Parent */
> -		close(pipefds[1]);
> +		close(out_pipefds[1]);
> +		close(in_pipefds[0]);
> +
>   		*childpid = pid;
> -		*fd = pipefds[0];
> +		if (out_fd)
> +			*out_fd = out_pipefds[0];
> +		if (in_fd)
> +			*in_fd = in_pipefds[1];
>
>   		return 0;
>   	}
> +
> +err_close_in:
> +	close(in_pipefds[0]);
> +	close(in_pipefds[1]);
> +err_close_out:
> +	close(out_pipefds[0]);
> +	close(out_pipefds[1]);
> +	return -1;
> +}
> +
> +/*
> + *  fwts_pipe_open_ro()
> + *	execl a command, return pid in *childpid and a pipe connected
> + *	to stdout in *fd. Return value < 0 indicates error.
> + */
> +int fwts_pipe_open_ro(const char *command, pid_t *childpid, int *fd)
> +{
> +	return fwts_pipe_open_rw(command, childpid, NULL, fd);
>   }
>
>   /*
>


Acked-by: Alex Hung <alex.hung at canonical.com>