ACK: [PATCH 3/6] pipeio: Rename fwts_pipe_open to fwts_pipe_open_ro, add fd as output argument

Alex Hung alex.hung at canonical.com
Wed Apr 27 04:31:25 UTC 2016


On 2016-04-22 01:41 PM, Jeremy Kerr wrote:
> In preparation for read/write pipe IO, this change renames
> fwts_pipe_open to fwts_pipe_open_ro, and adds fd as an output argument,
> instead of overloading the success/fail return status.
>
> This will allow us to add a _rw variant, which returns two file
> descriptors.
>
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
> ---
>   src/lib/include/fwts_pipeio.h |  2 +-
>   src/lib/src/fwts_cpu.c        |  2 +-
>   src/lib/src/fwts_devicetree.c |  4 ++--
>   src/lib/src/fwts_dump.c       |  2 +-
>   src/lib/src/fwts_pipeio.c     | 20 +++++++++++---------
>   5 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/src/lib/include/fwts_pipeio.h b/src/lib/include/fwts_pipeio.h
> index 96952e0..e27d90b 100644
> --- a/src/lib/include/fwts_pipeio.h
> +++ b/src/lib/include/fwts_pipeio.h
> @@ -30,7 +30,7 @@
>
>   #define FWTS_EXEC_ERROR		(127)
>
> -int   fwts_pipe_open(const char *command, pid_t *childpid);
> +int   fwts_pipe_open_ro(const char *command, pid_t *childpid, int *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_cpu.c b/src/lib/src/fwts_cpu.c
> index bcdb89d..a72500a 100644
> --- a/src/lib/src/fwts_cpu.c
> +++ b/src/lib/src/fwts_cpu.c
> @@ -64,7 +64,7 @@ int fwts_cpu_readmsr(const int cpu, const uint32_t reg, uint64_t *val)
>   	if ((fd = open(buffer, O_RDONLY)) < 0) {
>   		/* Hrm, msr not there, so force modprobe msr and see what happens */
>   		pid_t pid;
> -		if ((fd = fwts_pipe_open("modprobe msr", &pid)) < 0)
> +		if (fwts_pipe_open_ro("modprobe msr", &pid, &fd) < 0)
>   			return FWTS_ERROR;
>   		fwts_pipe_close(fd, pid);
>
> diff --git a/src/lib/src/fwts_devicetree.c b/src/lib/src/fwts_devicetree.c
> index b68a00e..baa4e6b 100644
> --- a/src/lib/src/fwts_devicetree.c
> +++ b/src/lib/src/fwts_devicetree.c
> @@ -39,8 +39,8 @@ int fwts_devicetree_read(fwts_framework *fwts)
>   	if (rc < 0)
>   		return FWTS_ERROR;
>
> -	fd = fwts_pipe_open(command, &pid);
> -	if (fd < 0) {
> +	rc = fwts_pipe_open_ro(command, &pid, &fd);
> +	if (rc < 0) {
>   		free(command);
>   		return FWTS_ERROR;
>   	}
> diff --git a/src/lib/src/fwts_dump.c b/src/lib/src/fwts_dump.c
> index edd2850..3ff888b 100644
> --- a/src/lib/src/fwts_dump.c
> +++ b/src/lib/src/fwts_dump.c
> @@ -91,7 +91,7 @@ static int dump_exec(const char *filename, const char *command)
>   	char *data;
>   	int ret;
>
> -	if ((fd = fwts_pipe_open(command, &pid)) < 0)
> +	if (fwts_pipe_open_ro(command, &pid, &fd) < 0)
>   		return FWTS_ERROR;
>
>   	if (fwts_pipe_read(fd, &data, &len) != 0) {
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index 7148903..64fc91d 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -40,11 +40,11 @@
>   #include "fwts.h"
>
>   /*
> - *  fwts_pipe_open()
> - *	execl a command, return pid in *childpid and
> - *	return fd. fd < 0 indicates error.
> + *  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(const char *command, pid_t *childpid)
> +int fwts_pipe_open_ro(const char *command, pid_t *childpid, int *fd)
>   {
>   	int pipefds[2];
>   	pid_t pid;
> @@ -78,15 +78,17 @@ int fwts_pipe_open(const char *command, pid_t *childpid)
>   		/* Parent */
>   		close(pipefds[1]);
>   		*childpid = pid;
> +		*fd = pipefds[0];
>
> -		return pipefds[0];
> +		return 0;
>   	}
>   }
>
>   /*
>    *  fwts_pipe_read()
> - *	read output from fwts_pipe_open(), *out_buf is populated with returned
> - *	data (allocated, must be free()-ed after use), and length in *out_len.
> + *	read output from fwts_pipe_open_ro(), *out_buf is populated with
> + *	returned data (allocated, must be free()-ed after use), and length in
> + *	*out_len.
>    *	Returns non-zero on failure.
>    */
>   int fwts_pipe_read(const int fd, char **out_buf, ssize_t *out_len)
> @@ -160,7 +162,7 @@ int fwts_pipe_exec(const char *command, fwts_list **list, int *status)
>   	ssize_t	len;
>   	char 	*text;
>
> -	if ((fd = fwts_pipe_open(command, &pid)) < 0)
> +	if (fwts_pipe_open_ro(command, &pid, &fd) < 0)
>   		return FWTS_ERROR;
>
>   	rc = fwts_pipe_read(fd, &text, &len);
> @@ -192,7 +194,7 @@ int fwts_exec(const char *command, int *status)
>   	pid_t 	pid;
>   	int	fd;
>
> -	if ((fd = fwts_pipe_open(command, &pid)) < 0)
> +	if (fwts_pipe_open_ro(command, &pid, &fd) < 0)
>   		return FWTS_ERROR;
>
>   	*status = fwts_pipe_close(fd, pid);
>

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



More information about the fwts-devel mailing list