Hi,

Ted Unangst wrote on Wed, Dec 30, 2015 at 01:22:17AM -0500:
> Philip Guenther wrote:
>> On Tue, Dec 29, 2015 at 2:34 PM, Todd C. Miller wrote:

>>> Since POSIX defers to ISO C we should be following the ISO C standard
>>> with respect to behavior when an encoding error occurs.  As such,
>>> I've changed my mind and now believe we should not be setting the
>>> error flag in those cases.

>> That sounds like it should be reported to austin-group so they can fix
>> the misleading wording.

> What do solaris and glibc do?  I'd expect the wording to be clarified
> in the direction of existing practice.

fgetwc(3):
FreeBSD sets the error flag since Oct 16, 2002 (rev. 105234).
NetBSD sets the error flag since July 3, 2006 (rev. 1.5), referencing SUSv3.
Dragonfly sets the error flag since April 21, 2009,
  (e0f95098eeba0176864b9cafe6d69b5b7bc0e73f), sync from FreeBSD.

fputwc(3):
FreeBSD sets the error flag since Oct 16, 2002 (rev. 105234).
NetBSD does NOT set the error flag.
Dragonfly sets the error flag since Sep 29, 2013,
  (0d5acd7467c4e95f792ef49fceb3ab8e917ce86b), sync from FreeBSD.


In don't understand the illumos code at all, and the glibc code
looks even worse, but i tested on these systems, and they set the
error flag for encoding errors in fgetwc(3):

SunOS unstable11s 5.11 11.2 sun4u sparc SUNW,SPARC-Enterprise
SunOS unstable10s 5.10 Generic_150400-17 sun4v sparc SUNW,SPARC-Enterprise-T5220
Linux donnerwolke.usta.de 3.16.0-4-686-pae #1 \
  SMP Debian 3.16.7-ckt11-1+deb8u3 (2015-08-04) i686 GNU/Linux

This one does not set the error flag:

SunOS unstable9s 5.9 Generic_Virtual sun4u sparc SUNW,SPARC-Enterprise-T5220

Solaris and Linux tests of fputwc(3) are inconclusive, i don't
understand how to set up invalid wchar_t values on Solaris and
Linux.


So, my conclusion is that it's the C standard that is carelessly
worded, not POSIX.  I don't think the C standard intends to say
that fgetwc(3) and fputwc(3) are not allowed to set the error
indicator on an encoding error, it just doesn't clearly say that
they are required to.  POSIX then adds the additional requirement,
merely failing to make it clear that it's resolving a careless
wording in the C standard.

The interpretation that setting the error indicator is deliberately
forbidden is not reasonable because that would make detection of
encoding errors fragile, complicated, and error-prone.

Assuming the POSIX specification, fgetwc(3) is easy and safe to
use:

  if (fgetwc(...) == WEOF) {  /* EOF or error */
    if (ferror(...))          /* error */
      inspect errno for the reason
    else                      /* EOF */
      ...
  }

Assuming setting the error indicator is forbidden, there is one way
of using the interface that is counter-intutive and fragile:

  if (fgetwc(...) == WEOF) {  /* EOF or error */
    if (feof(...))            /* EOF */
      ...
    else                      /* error, no matter what ferror(...) */
      inspect errno for the reason
  }

There is another way of using the error indicator that is safe,
but so contorted that we should not recommend it:

  int save_errno = errno;
  errno = 0;
  if (fgetwc(...) == WEOF) {  /* EOF or error */
    if (errno == EILSEQ)      /* encoding error */
      ...
    else if (ferror(...))     /* read error */
      inspect errno for the reason
    else                      /* EOF */
      ...
  }
  errno = save_errno


I think we should NOT revert the patch i committed that sets the
error indicator in fgetwc(3).  It's reasonable and agrees with what
everybody else does.  I think we should also commit the following
patch to fputwc(3), for the same reasons, and for consistency.

OK?
  Ingo


Index: fputwc.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/fputwc.c,v
retrieving revision 1.6
diff -u -p -r1.6 fputwc.c
--- fputwc.c    1 Oct 2015 02:32:07 -0000       1.6
+++ fputwc.c    10 Jan 2016 18:49:37 -0000
@@ -62,7 +62,7 @@ __fputwc_unlock(wchar_t wc, FILE *fp)
 
        size = wcrtomb(buf, wc, st);
        if (size == (size_t)-1) {
-               errno = EILSEQ;
+               fp->_flags |= __SERR;
                return WEOF;
        }
 

Reply via email to