ACK: [PATCH] lib: fix handling of -ve read returns

[Alex Hung]

On 07/14/2014 09:22 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Coverity Scan 7.5.0 picked up two identical issues where
> the -ve read returns were never being detected and so the
> code was not going to handle EINTR or EAGAIN errors
> correctly.  Re-worked the code in both places to work
> as intended.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/lib/src/fwts_acpi_tables.c | 22 ++++++++++++++--------
>   src/lib/src/fwts_pipeio.c      | 28 ++++++++++++++--------------
>   2 files changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
> index 291c59d..343f849 100644
> --- a/src/lib/src/fwts_acpi_tables.c
> +++ b/src/lib/src/fwts_acpi_tables.c
> @@ -598,26 +598,32 @@ static int fwts_acpi_load_tables_from_acpidump(fwts_framework *fw)
>   static uint8_t *fwts_acpi_load_table_from_file(const int fd, size_t *length)
>   {
>   	uint8_t *ptr = NULL;
> -	ssize_t n;
>   	size_t size = 0;
>   	char buffer[4096];
>
>   	*length = 0;
>
> -	while ((n = read(fd, buffer, sizeof(buffer))) > 0) {
> +	for (;;) {
> +		ssize_t n = read(fd, buffer, sizeof(buffer));
> +		uint8_t *tmp;
> +
> +		if (n == 0)
> +			break;
>   		if (n < 0) {
>   			if (errno != EINTR && errno != EAGAIN) {
>   				fwts_low_free(ptr);
>   				return NULL;
>   			}
> +			continue;
>   		}
> -		else {
> -			ptr = (uint8_t*)fwts_low_realloc(ptr, size + n + 1);
> -			if (ptr == NULL)
> -				return NULL;
> -			memcpy(ptr + size, buffer, n);
> -			size += n;
> +
> +		if ((tmp = (uint8_t*)fwts_low_realloc(ptr, size + n + 1)) == NULL) {
> +			free(ptr);
> +			return NULL;
>   		}
> +		ptr = tmp;
> +		memcpy(ptr + size, buffer, n);
> +		size += n;
>   	}
>   	*length = size;
>   	return ptr;
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index df07295..fe8e786 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -83,33 +83,33 @@ char *fwts_pipe_read(const int fd, ssize_t *length)
>   {
>   	char *ptr = NULL;
>   	char buffer[8192];
> -	ssize_t n;
>   	ssize_t size = 0;
>   	*length = 0;
>
>   	ptr = NULL;
>
> -	while ((n = read(fd, buffer, sizeof(buffer))) > 0) {
> +	for (;;) {
> +		ssize_t n = read(fd, buffer, sizeof(buffer));
> +		char *tmp;
> +
> +		if (n == 0)
> +			break;
>   		if (n < 0) {
>   			if (errno != EINTR && errno != EAGAIN) {
>   				free(ptr);
>   				return NULL;
>   			}
> +			continue;
>   		}
> -		else {
> -			char *new_ptr;
> -
> -			new_ptr = realloc(ptr, size + n + 1);
> -			if (new_ptr == NULL) {
> -				free(ptr);
> -				return NULL;
> -			} else
> -				ptr = new_ptr;
>
> -			memcpy(ptr + size, buffer, n);
> -			size += n;
> -			*(ptr+size) = 0;
> +		if ((tmp = realloc(ptr, size + n + 1)) == NULL) {
> +			free(ptr);
> +			return NULL;
>   		}
> +		ptr = tmp;
> +		memcpy(ptr + size, buffer, n);
> +		size += n;
> +		*(ptr+size) = 0;
>   	}
>   	*length = size;
>   	return ptr;
>

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