Re: [LTP] [PATCH 13/15] ujson: writer: Set errno when a flush fails

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> ujson_writer_file_close() uses errno to decide whether flushing the
> buffered data failed. out_writer_file_write() can fail without setting
> errno, when write() returns 0 or more bytes than requested, so close
> could report success even though data was lost. Set errno to EIO in
> these cases.
> 
> Fixes: c50a0e0751c8 ("libs: Vendor ujson library")
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  libs/ujson/ujson_writer.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libs/ujson/ujson_writer.c b/libs/ujson/ujson_writer.c
> index 15faca9be..49f73e436 100644
> --- a/libs/ujson/ujson_writer.c
> +++ b/libs/ujson/ujson_writer.c
> @@ -399,11 +399,15 @@ static int out_writer_file_write(ujson_writer *self, int fd, const char *buf, ss
>  		ssize_t ret = write(fd, buf, buf_len);
>  		if (ret <= 0) {
>  			err(self, "Failed to write to a file");
> +			/* ujson_writer_file_close() relies on errno */
> +			if (!ret)
> +				errno = EIO;

This is very edge case, most likely we will get ret == 0 if we are
writing to a non-blocking socket or pipe, however the code in the
library does only work on files. Maybe it would have more sense to add a
separtate message for the ret == 0 case:

		if (ret == 0) {
			err(self, "The write() syscall returned 0");
			errno = EAGAIN;
			return 1;
		}

>  			return 1;
>  		}
>  
>  		if (ret > buf_len) {
>  			err(self, "Wrote more bytes than requested?!");
> +			errno = EIO;
>  			return 1;
>  		}

This one looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp