"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: diff: Don't return errno when fread fails
To:
Tom Jones <thj@freebsd.org>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 11 Oct 2022 16:10:46 +0200

Download raw body.

Thread
On Tue, Oct 11, 2022 at 09:42:26AM +0100, Tom Jones wrote:
> fread doesn't consistently set errno on failure.
> 
> - On OpenBSD fread sets errno on possible argument overflows, but this
>   doesn't occur on other platforms. rfread doesn't set errno on EOF or
>   other failures.
> - ferror does not set errno on failure.
> 
> Returning errno here is possibly inconsistent. Return EIO here instead.
> 
> 
> - Tom

Make sense to me. ok

> diff --git a/lib/diff_atomize_text.c b/lib/diff_atomize_text.c
> index dd6b2aa..238ecb9 100644
> --- a/lib/diff_atomize_text.c
> +++ b/lib/diff_atomize_text.c
> @@ -64,7 +64,7 @@ diff_data_atomize_text_lines_fd(struct diff_data *d)
>  		while (eol == 0 && line_end < end) {
>  			r = fread(buf, sizeof(char), sizeof(buf), d->root->f);
>  			if (r == 0 && ferror(d->root->f))
> -				return errno;
> +				return EIO;
>  			i = 0;
>  			while (eol == 0 && i < r) {
>  				if (buf[i] != '\r' && buf[i] != '\n') {
> @@ -91,7 +91,7 @@ diff_data_atomize_text_lines_fd(struct diff_data *d)
>  				return errno;
>  			r = fread(buf, sizeof(char), sizeof(buf), d->root->f);
>  			if (r == 0 && ferror(d->root->f))
> -				return errno;
> +				return EIO;
>  			if (r > 0 && buf[0] == '\n')
>  				line_end++;
>  		}
> diff --git a/lib/diff_main.c b/lib/diff_main.c
> index 3043a9d..32c3dac 100644
> --- a/lib/diff_main.c
> +++ b/lib/diff_main.c
> @@ -43,7 +43,7 @@ read_at(FILE *f, off_t at_pos, unsigned char *buf, size_t len)
>  		return errno;
>  	r = fread(buf, sizeof(char), len, f);
>  	if ((r == 0 || r < len) && ferror(f))
> -		return errno;
> +		return EIO;
>  	if (r != len)
>  		return EIO;
>  	return 0;