ACK: [PATCH 1/6] pipeio: fix fwts_pipe_read return value when child produces no output

[Alex Hung]

On 2016-04-22 01:41 PM, Jeremy Kerr wrote:
> Currently, it's not possible to distinguish between a failure in
> fwts_pipe_read(), and the child process outputting no data - in both
> cases, NULL is returned.
>
> This change moves the returned buffer to an output argument, with the
> return value indicating errors.
>
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
> ---
>   src/lib/include/fwts_pipeio.h |  2 +-
>   src/lib/src/fwts_devicetree.c |  6 +++---
>   src/lib/src/fwts_dump.c       |  2 +-
>   src/lib/src/fwts_pipeio.c     | 21 +++++++++++----------
>   4 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/src/lib/include/fwts_pipeio.h b/src/lib/include/fwts_pipeio.h
> index b014587..96952e0 100644
> --- a/src/lib/include/fwts_pipeio.h
> +++ b/src/lib/include/fwts_pipeio.h
> @@ -31,7 +31,7 @@
>   #define FWTS_EXEC_ERROR		(127)
>
>   int   fwts_pipe_open(const char *command, pid_t *childpid);
> -char *fwts_pipe_read(const int fd, ssize_t *length);
> +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);
>   int   fwts_exec(const char *command, int *status);
> diff --git a/src/lib/src/fwts_devicetree.c b/src/lib/src/fwts_devicetree.c
> index 41cee54..b68a00e 100644
> --- a/src/lib/src/fwts_devicetree.c
> +++ b/src/lib/src/fwts_devicetree.c
> @@ -46,15 +46,15 @@ int fwts_devicetree_read(fwts_framework *fwts)
>   	}
>   	free(command);
>
> -	data = fwts_pipe_read(fd, &len);
> -	if (data == NULL) {
> +	rc = fwts_pipe_read(fd, &data, &len);
> +	if (rc) {
>   		fwts_pipe_close(fd, pid);
>   		return FWTS_ERROR;
>   	}
>
>   	status = fwts_pipe_close(fd, pid);
>
> -	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
> +	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0 || len == 0) {
>   		fprintf(stderr, "Cannot read devicetree data: dtc failed\n");
>   		return FWTS_ERROR;
>   	}
> diff --git a/src/lib/src/fwts_dump.c b/src/lib/src/fwts_dump.c
> index b348d6f..edd2850 100644
> --- a/src/lib/src/fwts_dump.c
> +++ b/src/lib/src/fwts_dump.c
> @@ -94,7 +94,7 @@ static int dump_exec(const char *filename, const char *command)
>   	if ((fd = fwts_pipe_open(command, &pid)) < 0)
>   		return FWTS_ERROR;
>
> -	if ((data = fwts_pipe_read(fd, &len)) == NULL) {
> +	if (fwts_pipe_read(fd, &data, &len) != 0) {
>   		fwts_pipe_close(fd, pid);
>   		return FWTS_ERROR;
>   	}
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index 7dda976..75addc5 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -85,16 +85,16 @@ int fwts_pipe_open(const char *command, pid_t *childpid)
>
>   /*
>    *  fwts_pipe_read()
> - *	read output from fwts_pipe_open(), *length is
> - *	set to the number of chars read and we return
> - *	a buffer of read data.
> + *	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.
> + *	Returns non-zero on failure.
>    */
> -char *fwts_pipe_read(const int fd, ssize_t *length)
> +int fwts_pipe_read(const int fd, char **out_buf, ssize_t *out_len)
>   {
>   	char *ptr = NULL;
>   	char buffer[8192];
>   	ssize_t size = 0;
> -	*length = 0;
> +	*out_len = 0;
>
>   	ptr = NULL;
>
> @@ -107,22 +107,23 @@ char *fwts_pipe_read(const int fd, ssize_t *length)
>   		if (n < 0) {
>   			if (errno != EINTR && errno != EAGAIN) {
>   				free(ptr);
> -				return NULL;
> +				return -1;
>   			}
>   			continue;
>   		}
>
>   		if ((tmp = realloc(ptr, size + n + 1)) == NULL) {
>   			free(ptr);
> -			return NULL;
> +			return -1;
>   		}
>   		ptr = tmp;
>   		memcpy(ptr + size, buffer, n);
>   		size += n;
>   		*(ptr+size) = 0;
>   	}
> -	*length = size;
> -	return ptr;
> +	*out_len = size;
> +	*out_buf = ptr;
> +	return 0;
>   }
>
>   /*
> @@ -162,7 +163,7 @@ int fwts_pipe_exec(const char *command, fwts_list **list, int *status)
>   	if ((fd = fwts_pipe_open(command, &pid)) < 0)
>   		return FWTS_ERROR;
>
> -	text = fwts_pipe_read(fd, &len);
> +	fwts_pipe_read(fd, &text, &len);
>   	*list = fwts_list_from_text(text);
>   	free(text);
>
>


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