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.
Test program:
#include <err.h>
#include <errno.h>
#include <stdio.h>
#include <wchar.h>
int
main(void)
{
wchar_t ws[2];
int irc;
ws[0] = 0xdc00; /* UTF-16 surrogate, invalid code point. */
ws[1] = L'\0';
irc = printf("%ls\n", ws);
fprintf(stderr, "printf(ws) returned %d\n", irc);
fprintf(stderr, "error status %d\n", ferror(stdout));
warn("printf(ws)");
clearerr(stdout);
fprintf(stderr, "\nafter clearerr: error status %d\n\n",
ferror(stdout));
errno = 0;
irc = printf("test\n");
fprintf(stderr, "printf(test) returned %d\n", irc);
fprintf(stderr, "error status %d\n", ferror(stdout));
warn("printf(ws)");
return 0;
}
Output before:
$ ./printf
printf(ws) returned -1
error status 1
printf: printf(ws): Illegal byte sequence
after clearerr: error status 0
printf(test) returned -1
error status 0
printf: printf(ws): Bad file descriptor
Notice that the "test" wasn't printed.
Output with the patch below:
$ ./printf
printf(ws) returned -1
error status 0
printf: printf(ws): Illegal byte sequence
after clearerr: error status 0
test
printf(test) returned 5
error status 0
printf: printf(ws): Undefined error: 0
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?
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;
