mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH v2] fgetws: fix checking for fgetwc errors
@ 2022-02-10  3:08 Pinghao Wu
  2022-02-20  0:57 ` Rich Felker
  0 siblings, 1 reply; 3+ messages in thread
From: Pinghao Wu @ 2022-02-10  3:08 UTC (permalink / raw)
  To: musl; +Cc: Pinghao Wu

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH v2] fgetws: fix checking for fgetwc errors
  2022-02-10  3:08 [musl] [PATCH v2] fgetws: fix checking for fgetwc errors Pinghao Wu
@ 2022-02-20  0:57 ` Rich Felker
  2022-02-20  3:54   ` xdavidwu
  0 siblings, 1 reply; 3+ messages in thread
From: Rich Felker @ 2022-02-20  0:57 UTC (permalink / raw)
  To: Pinghao Wu; +Cc: musl

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] [PATCH v2] fgetws: fix checking for fgetwc errors
  2022-02-20  0:57 ` Rich Felker
@ 2022-02-20  3:54   ` xdavidwu
  0 siblings, 0 replies; 3+ messages in thread
From: xdavidwu @ 2022-02-20  3:54 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl

> 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?
Yes, I think this is the best solution. 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-02-20  3:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10  3:08 [musl] [PATCH v2] fgetws: fix checking for fgetwc errors Pinghao Wu
2022-02-20  0:57 ` Rich Felker
2022-02-20  3:54   ` xdavidwu

Code repositories for project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).