Ingo Schwarze <schwa...@usta.de> writes:
> Fourth file, fourth broken file.
> This is the worst bug found so far.
> All i'm doing is grepping libc/stdio for "EILSEQ".
> So far, every single instance i looked at was buggy.
>
> I think we should cvs rm libc.
> The code quality just isn't up to OpwnBSD standards.
>
>
> When fprintf(fp, "...%ls...", ...) encounters an encoding error,
> it trashes fp BEYOND REPAIR, clearing all FILE flags, making the
> file unreadable, unwriteable, and probably breaking even more than
> that. Calling clearerr(3) is by far insufficient to undo the
> devastating damage done.
>
> Besides, i don't see the point in messing with FILE flags at all
> in case of encoding errors. As opposed to fgetwc(3) and fputwc(3),
> the manual doesn't document this fiddling, and POSIX doesn't ask
> for it. No other conversions in printf(3) set the error indicator.
> It isn't required because printf(3) provides a proper error return
> (-1) in the first place. Has anybody ever seen any code calling
> ferror(3) after printf(3)? That just wouldn't make sense.
> Of course, printf(3) can result in the error indicator getting set,
> but only if the underlying low-level write calls fail.
>
> So, don't fp->_flags = __SERR; (sic, no |= here!),
> don't even fp->_flags |= __SERR;.
>
> While here, as usual, remove the pointless and slightly dangerous
> errno = EILSEQ overrides after functions that already do that
> and are required by the standard to do so.
[...]
> Note that all code changes below are in parts guarded with
> #ifdef PRINTF_WIDE_CHAR
> so there is no risk of breaking printf(3) at large.
>
> OK?
Makes sense to me, ok jca@
(I'll review your next diff later.)
> Ingo
>
>
> Index: stdio/vfprintf.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdio/vfprintf.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 vfprintf.c
> --- stdio/vfprintf.c 29 Sep 2015 03:19:24 -0000 1.69
> +++ stdio/vfprintf.c 24 Dec 2015 22:58:33 -0000
> @@ -165,10 +165,8 @@ __wcsconv(wchar_t *wcsarg, int prec)
> memset(&mbs, 0, sizeof(mbs));
> p = wcsarg;
> nbytes = wcsrtombs(NULL, (const wchar_t **)&p, 0, &mbs);
> - if (nbytes == (size_t)-1) {
> - errno = EILSEQ;
> + if (nbytes == (size_t)-1)
> return (NULL);
> - }
> } else {
> /*
> * Optimisation: if the output precision is small enough,
> @@ -188,10 +186,8 @@ __wcsconv(wchar_t *wcsarg, int prec)
> break;
> nbytes += clen;
> }
> - if (clen == (size_t)-1) {
> - errno = EILSEQ;
> + if (clen == (size_t)-1)
> return (NULL);
> - }
> }
> }
> if ((convbuf = malloc(nbytes + 1)) == NULL)
> @@ -203,7 +199,6 @@ __wcsconv(wchar_t *wcsarg, int prec)
> if ((nbytes = wcsrtombs(convbuf, (const wchar_t **)&p,
> nbytes, &mbs)) == (size_t)-1) {
> free(convbuf);
> - errno = EILSEQ;
> return (NULL);
> }
> convbuf[nbytes] = '\0';
> @@ -640,8 +635,7 @@ reswitch: switch (ch) {
> mbseqlen = wcrtomb(buf,
> (wchar_t)GETARG(wint_t), &mbs);
> if (mbseqlen == (size_t)-1) {
> - fp->_flags |= __SERR;
> - errno = EILSEQ;
> + ret = -1;
> goto error;
> }
> cp = buf;
> @@ -855,7 +849,7 @@ fp_common:
> } else {
> convbuf = __wcsconv(wcp, prec);
> if (convbuf == NULL) {
> - fp->_flags = __SERR;
> + ret = -1;
> goto error;
> }
> cp = convbuf;
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
