From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 3335 invoked from network); 20 Feb 2022 00:58:08 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 20 Feb 2022 00:58:08 -0000 Received: (qmail 1725 invoked by uid 550); 20 Feb 2022 00:58:04 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 1687 invoked from network); 20 Feb 2022 00:58:03 -0000 Date: Sat, 19 Feb 2022 19:57:50 -0500 From: Rich Felker To: Pinghao Wu Cc: musl@lists.openwall.com Message-ID: <20220220005749.GW7074@brightrain.aerifal.cx> References: <20220210030813.1943-1-xdavidwuph@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220210030813.1943-1-xdavidwuph@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH v2] fgetws: fix checking for fgetwc errors 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