Re: [musl] [PATCH v2] fgetws: fix checking for fgetwc errors

[Rich Felker <dalias@libc.org>]

On Thu, Feb 10, 2022 at 11:08:13AM +0800, Pinghao Wu wrote:
> This corrects checking for fgetwc errors by arming dummy errno before
> each invocation, and checking errors only if it returns WEOF.
> ---
> v2: move error checking into the loop.
> 
>  src/stdio/fgetws.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/stdio/fgetws.c b/src/stdio/fgetws.c
> index b08b3049..cffff84a 100644
> --- a/src/stdio/fgetws.c
> +++ b/src/stdio/fgetws.c
> @@ -12,18 +12,22 @@ wchar_t *fgetws(wchar_t *restrict s, int n, FILE *restrict f)
>  
>  	FLOCK(f);
>  
> -	/* Setup a dummy errno so we can detect EILSEQ. This is
> -	 * the only way to catch encoding errors in the form of a
> -	 * partial character just before EOF. */
> -	errno = EAGAIN;
> +	int err = 0;
>  	for (; n; n--) {
> +		/* Setup a dummy errno so we can detect EILSEQ. This is
> +		 * the only way to catch encoding errors in the form of a
> +		 * partial character just before EOF. */
> +		errno = EAGAIN;
>  		wint_t c = __fgetwc_unlocked(f);
> -		if (c == WEOF) break;
> +		if (c == WEOF) {
> +			if (ferror(f) || errno == EILSEQ) err = 1;
> +			break;
> +		}
>  		*p++ = c;
>  		if (c == '\n') break;
>  	}
>  	*p = 0;
> -	if (ferror(f) || errno==EILSEQ) p = s;
> +	if (err) p = s;
>  
>  	FUNLOCK(f);
>  
> -- 
> 2.35.1

Thanks, and sorry for taking a while to follow up on this. It turns
out there was information we were missing. The logic that's there in
fgetws, from commit a90d9da1d1b14d81c4f93e1a6d1a686c3312e4ba, predates
changes by commit 511d70738bce11a67219d0132ce725c323d00e4e to fgetwc.
As noted there (and also in Austin Group bug 1022, comment 4383), ISO
C has been amended to require the error indicator to be set for the
stream on encoding errors.

This makes the stuff fgetws is doing to try to disambiguate the result
of fgetwc obsolete. We no longer need to poke at errno, just check
ferror. I think that means we can just revert
a90d9da1d1b14d81c4f93e1a6d1a686c3312e4ba. Does that look right?

Rich