Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Andreas Köpke]

Hi folks, thanks for the great work. 

I'm working on bare metal using newlib - with the changes to perror it does 
not compile anymore: none of the headers (sys/uio.h, unistd.h) are known.

So I guess we need some split here.

Thanks, Andreas 

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

Hi,

On Thu, 5 Jul 2018 13:38:52 +0200
Andreas Köpke wrote:
> I'm working on bare metal using newlib - with the changes to perror it does 
> not compile anymore: none of the headers (sys/uio.h, unistd.h) are known.

Bare metal does not have not only sys/uio.h but also unistd.h?
It seems a few stdio files includes unistd.h at least for several
months ago.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Andreas Köpke]

ah sorry -- unistd.h works. 

> Hi,
> 
> On Thu, 5 Jul 2018 13:38:52 +0200
> 
> Andreas Köpke wrote:
> > I'm working on bare metal using newlib - with the changes to perror it
> > does
> > not compile anymore: none of the headers (sys/uio.h, unistd.h) are known.
> 
> Bare metal does not have not only sys/uio.h but also unistd.h?
> It seems a few stdio files includes unistd.h at least for several
> months ago.

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

On Jul  5 15:36, Jeff Johnston wrote:
> I have checked in this revised patch to mitigate the reported build
> breakages.  Corinna, feel free to still comment.

No, that's ok.  We're at least FreeBSD compatible now.

Thanks,
Corinna

> 
> -- Jeff J.
> 
> On Thu, Jul 5, 2018 at 10:07 AM, Takashi Yano <takashi.yano@nifty.ne.jp>
> wrote:
> 
> > On Thu, 5 Jul 2018 14:49:00 +0200
> > Corinna Vinschen wrote:
> > > ...I noticed that FreeBSD handles psignal differently.  It writes the
> > > output immediately to STDERR_FILENO, rather than to stderr or
> > > fileno(stderr).  It does also not call fflush or reset the _SOFF flag.
> > ...
> > > Thoughts?
> >
> > Since _newlib_flockfile_start()/end() are defined in stdio/local.h, using
> > _newlib_flockfile_start()/end() from psignal.c is a bit painful. Moreover
> > there are several reports that they can not build newlib on bare metal.
> > Therefore, I imitated FreeBSD in a hurry just because it seems the easiest
> > way.
> >
> > --
> > Takashi Yano <takashi.yano@nifty.ne.jp>
> >

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Craig Howland]

On 07/05/2018 08:49 AM, Corinna Vinschen wrote:
> On Jul  5 20:35, Takashi Yano wrote:
>> Hi Corinna,
>>
>> Thank you for checking.
>>
>> On Thu, 5 Jul 2018 13:19:21 +0200
>> Corinna Vinschen wrote:
>>> Yes, we need it, the entire operation, flushing and writing, must be
>>> cancel-safe and synchronized with other access to the stderr FILE.
>>>
>>> Comparing with FreeBSD, there's also something missing.  After the
>>> write operation, the offset in the FILE structure is incorrect.
>>> Consequentially the __SOFF flag is reset to 0 last thing before
>>> unlocking the file:
>>>
>>>    stderr->_flags &= ~__SOFF;
>> Do you mean these are necessary for both perror.c and psignal.c?
>> How about psiginfo() case?
> Well, as long as we use the method at hand, I think it's the right thing
> to do.  However...
>
> ...I noticed that FreeBSD handles psignal differently.  It writes the
> output immediately to STDERR_FILENO, rather than to stderr or
> fileno(stderr).  It does also not call fflush or reset the _SOFF flag.
>
> OpenBSD uses this method for perror as well, no syncing with stderr,
> no cancel-safety.
>
> GLibc duplicates the file descriptor in perror, but writes immediately
> to STDERR_FILENO in psiginfo, or uses an internal variation of
> fprintf in psignal (thus potentially changing the orientation).
>
> Ok, I'm a bit more puzzled than before.  What all this is missing is
> *consistency*.
>
> I really wonder how standards-compliant this is.  For all three
> functions POSIX says something like "... shall print a message out on
> stderr ...".  stderr is the stream and it's underlying descriptor, not
> necessarily STDERR_FILENO.
>
> If immediate writing to STDERR_FILENO is ok, we should simply call
> write(STDERR_FILENO) and be done with it.  Just like BSDs and Linux,
> we don't even have to loop until all bytes have been written, none
> of the above does.
>
>
> Thoughts?
>
>
> Corinna
>

      POSIX defines all 3 functions with the same basic requirements, so it 
certainly does seem like whatever we do ought to be done in the same way for all 
3.  (I suppose you could make an argument that perror() is C while the other 2 
are not, allowing a possible difference there.  But that doesn't seem like a 
good idea.)
      I think that we're kind of stuck needing the fflush().  While I'd rather 
just skip it, not calling it can provide end behavior that is clearly different 
than perror()/etc. writing to the stream, itself, instead of calling write(2) 
(even if only when it is not unbuffered).    That is, with a line or 
fully-buffered stream, anything in the stream buffer already at the time 
perror() is called should appear before the perror() output.  If we don't flush, 
the in-buffer data appear after.  The fflush() is what ends up making it look 
like we wrote to the stream using the more usual mechanism instead of write 
directly.  That is, I think that with the fflush(), the end result should be the 
same as if we really were writing through the stream mechanism.  (Although 
there's a complication related to timing, which is why I'm saying "end result" 
instead of "identical results."  See below.)  This means we would be statically 
standard compliant, even if the underlying mechanism is doing an end run around 
the usual internal stream write mechanisms.
      To put this into a different form:

lock stream stderr;
fflush(stderr);
write(fileno(stderr), string);
unlock stream stderr;

should produce the same end effect as

fputs(stderr, string);   /* (effectively what perror() is required to end up 
doing) */

in terms of the sequence of bytes which end up in the file.
      The former can produce a clear timing difference, however, although only 
if the stream is fully buffered:  the fputs() output might not appear until 
something else is written (depending upon where the buffer fill happens to be), 
while the former case forces string out right away.  This seems impossible to 
fix without returning to the full streams mechanism, but then we're back to the 
complex 'don't change the orientation' problem we're trying to avoid.  It seems 
a small enough problem to accept, especially given the mess you're describing 
for the other libraries.
      Along the same lines, since they are supposed to write to the stream, 
strictly fileno(stderr) should be used, since it is possible for STDERR_FILENO 
(a fixed constant) to be inaccurate.  This point really is independent of the 
fflush() question.  (At least something seems easy to answer.)
                 Craig

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Jeff Johnston]

I have checked in this revised patch to mitigate the reported build
breakages.  Corinna, feel free to still comment.

-- Jeff J.

On Thu, Jul 5, 2018 at 10:07 AM, Takashi Yano <takashi.yano@nifty.ne.jp>
wrote:

> On Thu, 5 Jul 2018 14:49:00 +0200
> Corinna Vinschen wrote:
> > ...I noticed that FreeBSD handles psignal differently.  It writes the
> > output immediately to STDERR_FILENO, rather than to stderr or
> > fileno(stderr).  It does also not call fflush or reset the _SOFF flag.
> ...
> > Thoughts?
>
> Since _newlib_flockfile_start()/end() are defined in stdio/local.h, using
> _newlib_flockfile_start()/end() from psignal.c is a bit painful. Moreover
> there are several reports that they can not build newlib on bare metal.
> Therefore, I imitated FreeBSD in a hurry just because it seems the easiest
> way.
>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>
>

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On Thu, 5 Jul 2018 14:49:00 +0200
Corinna Vinschen wrote:
> ...I noticed that FreeBSD handles psignal differently.  It writes the
> output immediately to STDERR_FILENO, rather than to stderr or
> fileno(stderr).  It does also not call fflush or reset the _SOFF flag.
...
> Thoughts?

Since _newlib_flockfile_start()/end() are defined in stdio/local.h, using
_newlib_flockfile_start()/end() from psignal.c is a bit painful. Moreover
there are several reports that they can not build newlib on bare metal.
Therefore, I imitated FreeBSD in a hurry just because it seems the easiest
way.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-Fix-newlib-functions-perror-psignal-not-to-use-write.patch --]
[-- Type: application/octet-stream, Size: 3623 bytes --]

From 598ebd7885da2bf1bb3dfc7fc60977d74b47caed Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Thu, 5 Jul 2018 23:01:26 +0900
Subject: [PATCH] Fix newlib functions perror()/psignal() not to use writev().

This fix is for some platforms which do not have writev().
*perror.c: Use _write_r() instead of writev().
*psignal.c: Use write() insetad of writev().

Revise commit: d4f4e7ae1be1bcf8c021f2b0865aafc16b338aa3
---
 newlib/libc/signal/psignal.c | 36 ++++++++++++++++----------------
 newlib/libc/stdio/perror.c   | 40 +++++++++++++++++++-----------------
 2 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/newlib/libc/signal/psignal.c b/newlib/libc/signal/psignal.c
index 9a584869d..f847ab2c8 100644
--- a/newlib/libc/signal/psignal.c
+++ b/newlib/libc/signal/psignal.c
@@ -32,37 +32,37 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <_ansi.h>
 #include <stdio.h>
 #include <string.h>
-#include <sys/uio.h>
+#include <unistd.h>
 
-#define ADD(str) \
+#define WRITE_STR(str) \
 { \
-  v->iov_base = (void *)(str); \
-  v->iov_len = strlen (v->iov_base); \
-  v ++; \
-  iov_cnt ++; \
+  const char *p = (str); \
+  size_t len = strlen (p); \
+  while (len) \
+    { \
+      ssize_t len1 = write (fileno (stderr), p, len); \
+      if (len1 < 0) \
+	break; \
+      len -= len1; \
+      p += len1; \
+    } \
 }
 
 void
 psignal (int sig,
        const char *s)
 {
-  struct iovec iov[4];
-  struct iovec *v = iov;
-  int iov_cnt = 0;
-
+  fflush (stderr);
   if (s != NULL && *s != '\0')
     {
-      ADD (s);
-      ADD (": ");
+      WRITE_STR (s);
+      WRITE_STR (": ");
     }
-  ADD (strsignal (sig));
+  WRITE_STR (strsignal (sig));
 
 #ifdef __SCLE
-  ADD ((stderr->_flags & __SCLE) ? "\r\n" : "\n");
+  WRITE_STR ((stderr->_flags & __SCLE) ? "\r\n" : "\n");
 #else
-  ADD ("\n");
+  WRITE_STR ("\n");
 #endif
-
-  fflush (stderr);
-  writev (fileno (stderr), iov, iov_cnt);
 }
diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index 831c67eef..68d78c227 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -56,15 +56,20 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <reent.h>
 #include <stdio.h>
 #include <string.h>
-#include <sys/uio.h>
 #include "local.h"
 
-#define ADD(str) \
+#define WRITE_STR(str) \
 { \
-  v->iov_base = (void *)(str); \
-  v->iov_len = strlen (v->iov_base); \
-  v ++; \
-  iov_cnt ++; \
+  const char *p = (str); \
+  size_t len = strlen (p); \
+  while (len) \
+    { \
+      ssize_t len1 = _write_r (ptr, fileno (fp), p, len); \
+      if (len1 < 0) \
+	break; \
+      len -= len1; \
+      p += len1; \
+    } \
 }
 
 void
@@ -73,31 +78,28 @@ _perror_r (struct _reent *ptr,
 {
   char *error;
   int dummy;
-  struct iovec iov[4];
-  struct iovec *v = iov;
-  int iov_cnt = 0;
   FILE *fp = _stderr_r (ptr);
 
   CHECK_INIT (ptr, fp);
+
+  _newlib_flockfile_start(fp);
+  _fflush_r (ptr, fp);
   if (s != NULL && *s != '\0')
     {
-      ADD (s);
-      ADD (": ");
+      WRITE_STR (s);
+      WRITE_STR (": ");
     }
 
   if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
-    ADD (error);
+    WRITE_STR (error);
 
 #ifdef __SCLE
-  ADD ((fp->_flags & __SCLE) ? "\r\n" : "\n");
+  WRITE_STR ((fp->_flags & __SCLE) ? "\r\n" : "\n");
 #else
-  ADD ("\n");
+  WRITE_STR ("\n");
 #endif
-
-  _newlib_flockfile_start (fp);
-  fflush (fp);
-  writev (fileno (fp), iov, iov_cnt);
-  _newlib_flockfile_end (fp);
+  fp->_flags &= ~__SOFF;
+  _newlib_flockfile_end(fp);
 }
 
 #ifndef _REENT_ONLY
-- 
2.17.0

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

On Jul  5 20:35, Takashi Yano wrote:
> Hi Corinna,
> 
> Thank you for checking.
> 
> On Thu, 5 Jul 2018 13:19:21 +0200
> Corinna Vinschen wrote:
> > Yes, we need it, the entire operation, flushing and writing, must be
> > cancel-safe and synchronized with other access to the stderr FILE.
> > 
> > Comparing with FreeBSD, there's also something missing.  After the
> > write operation, the offset in the FILE structure is incorrect.
> > Consequentially the __SOFF flag is reset to 0 last thing before
> > unlocking the file:
> > 
> >   stderr->_flags &= ~__SOFF;
> 
> Do you mean these are necessary for both perror.c and psignal.c?
> How about psiginfo() case?

Well, as long as we use the method at hand, I think it's the right thing
to do.  However...

...I noticed that FreeBSD handles psignal differently.  It writes the
output immediately to STDERR_FILENO, rather than to stderr or
fileno(stderr).  It does also not call fflush or reset the _SOFF flag.

OpenBSD uses this method for perror as well, no syncing with stderr,
no cancel-safety.

GLibc duplicates the file descriptor in perror, but writes immediately
to STDERR_FILENO in psiginfo, or uses an internal variation of
fprintf in psignal (thus potentially changing the orientation).

Ok, I'm a bit more puzzled than before.  What all this is missing is
*consistency*.

I really wonder how standards-compliant this is.  For all three
functions POSIX says something like "... shall print a message out on
stderr ...".  stderr is the stream and it's underlying descriptor, not
necessarily STDERR_FILENO.

If immediate writing to STDERR_FILENO is ok, we should simply call
write(STDERR_FILENO) and be done with it.  Just like BSDs and Linux,
we don't even have to loop until all bytes have been written, none
of the above does.


Thoughts?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

Hi Corinna,

Thank you for checking.

On Thu, 5 Jul 2018 13:19:21 +0200
Corinna Vinschen wrote:
> Yes, we need it, the entire operation, flushing and writing, must be
> cancel-safe and synchronized with other access to the stderr FILE.
> 
> Comparing with FreeBSD, there's also something missing.  After the
> write operation, the offset in the FILE structure is incorrect.
> Consequentially the __SOFF flag is reset to 0 last thing before
> unlocking the file:
> 
>   stderr->_flags &= ~__SOFF;

Do you mean these are necessary for both perror.c and psignal.c?
How about psiginfo() case?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

On Jul  5 19:41, Takashi Yano wrote:
> Hi Corinna,
> 
> On Thu, 5 Jul 2018 11:08:51 +0200
> Corinna Vinschen wrote:
> > Ouch!  I didn't realize that writev is not a required function on
> > bare metal, sorry.
> > 
> > Takashi, we need a patch to implement perror/psignal without writev, for
> > instance by calling write twice.  Care to send a followup patch?
> 
> Patch attached.
> 
> I am not sure whether _newlib_flockfile_start()/end() is necessary or not.
> Could you please check?

Yes, we need it, the entire operation, flushing and writing, must be
cancel-safe and synchronized with other access to the stderr FILE.

Comparing with FreeBSD, there's also something missing.  After the
write operation, the offset in the FILE structure is incorrect.
Consequentially the __SOFF flag is reset to 0 last thing before
unlocking the file:

  stderr->_flags &= ~__SOFF;

Also:

> +#define WRITE_STR(str) \
>  { \
> -  v->iov_base = (void *)(str); \
> -  v->iov_len = strlen (v->iov_base); \
> -  v ++; \
> -  iov_cnt ++; \
> +  const char *p = (str); \
> +  size_t len = strlen (p); \
> +  while (len) \
> +    { \
> +      ssize_t len1 = write (fileno (stderr), p, len); \
> +      if (len1 < 0) break; \

Please put the break on a line of its own:

   if (len1 < 0) \
     break; \

> [...]
> +#define WRITE_STR(str) \
>  { \
> -  v->iov_base = (void *)(str); \
> -  v->iov_len = strlen (v->iov_base); \
> -  v ++; \
> -  iov_cnt ++; \
> +  const char *p = (str); \
> +  size_t len = strlen (p); \
> +  while (len) \
> +    { \
> +      ssize_t len1 = _write_r (ptr, fileno (fp), p, len); \
> +      if (len1 < 0) break; \

Ditto.

> +      len -= len1; \
> +      p += len1; \
> +    } \
>  }
>  
>  void
> @@ -73,31 +77,25 @@ _perror_r (struct _reent *ptr,
>  {
>    char *error;
>    int dummy;
> -  struct iovec iov[4];
> -  struct iovec *v = iov;
> -  int iov_cnt = 0;
>    FILE *fp = _stderr_r (ptr);
>  
>    CHECK_INIT (ptr, fp);
> +
> +  fflush (fp);

I only just noticed, sorry.  Please call

     _fflush_r (ptr, fp);


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

Hi Corinna,

On Thu, 5 Jul 2018 11:08:51 +0200
Corinna Vinschen wrote:
> Ouch!  I didn't realize that writev is not a required function on
> bare metal, sorry.
> 
> Takashi, we need a patch to implement perror/psignal without writev, for
> instance by calling write twice.  Care to send a followup patch?

Patch attached.

I am not sure whether _newlib_flockfile_start()/end() is necessary or not.
Could you please check?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-Fix-newlib-functions-perror-psignal-not-to-use-write.patch --]
[-- Type: application/octet-stream, Size: 3524 bytes --]

From 5c4ad606da5fae71fa8e0cd78a265829260cdb64 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Thu, 5 Jul 2018 19:22:05 +0900
Subject: [PATCH] Fix newlib functions perror()/psignal() not to use writev().

This fix is for some platforms which does not have writev().
*perror.c: Use _write_r() instead of writev().
*psignal.c: Use write() insetad of writev().

Revise commit: d4f4e7ae1be1bcf8c021f2b0865aafc16b338aa3
---
 newlib/libc/signal/psignal.c | 35 +++++++++++++++++------------------
 newlib/libc/stdio/perror.c   | 36 +++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/newlib/libc/signal/psignal.c b/newlib/libc/signal/psignal.c
index 9a584869d..522fc79f2 100644
--- a/newlib/libc/signal/psignal.c
+++ b/newlib/libc/signal/psignal.c
@@ -32,37 +32,36 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <_ansi.h>
 #include <stdio.h>
 #include <string.h>
-#include <sys/uio.h>
+#include <unistd.h>
 
-#define ADD(str) \
+#define WRITE_STR(str) \
 { \
-  v->iov_base = (void *)(str); \
-  v->iov_len = strlen (v->iov_base); \
-  v ++; \
-  iov_cnt ++; \
+  const char *p = (str); \
+  size_t len = strlen (p); \
+  while (len) \
+    { \
+      ssize_t len1 = write (fileno (stderr), p, len); \
+      if (len1 < 0) break; \
+      len -= len1; \
+      p += len1; \
+    } \
 }
 
 void
 psignal (int sig,
        const char *s)
 {
-  struct iovec iov[4];
-  struct iovec *v = iov;
-  int iov_cnt = 0;
-
+  fflush (stderr);
   if (s != NULL && *s != '\0')
     {
-      ADD (s);
-      ADD (": ");
+      WRITE_STR (s);
+      WRITE_STR (": ");
     }
-  ADD (strsignal (sig));
+  WRITE_STR (strsignal (sig));
 
 #ifdef __SCLE
-  ADD ((stderr->_flags & __SCLE) ? "\r\n" : "\n");
+  WRITE_STR ((stderr->_flags & __SCLE) ? "\r\n" : "\n");
 #else
-  ADD ("\n");
+  WRITE_STR ("\n");
 #endif
-
-  fflush (stderr);
-  writev (fileno (stderr), iov, iov_cnt);
 }
diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index 831c67eef..9527939ee 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -56,15 +56,19 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <reent.h>
 #include <stdio.h>
 #include <string.h>
-#include <sys/uio.h>
 #include "local.h"
 
-#define ADD(str) \
+#define WRITE_STR(str) \
 { \
-  v->iov_base = (void *)(str); \
-  v->iov_len = strlen (v->iov_base); \
-  v ++; \
-  iov_cnt ++; \
+  const char *p = (str); \
+  size_t len = strlen (p); \
+  while (len) \
+    { \
+      ssize_t len1 = _write_r (ptr, fileno (fp), p, len); \
+      if (len1 < 0) break; \
+      len -= len1; \
+      p += len1; \
+    } \
 }
 
 void
@@ -73,31 +77,25 @@ _perror_r (struct _reent *ptr,
 {
   char *error;
   int dummy;
-  struct iovec iov[4];
-  struct iovec *v = iov;
-  int iov_cnt = 0;
   FILE *fp = _stderr_r (ptr);
 
   CHECK_INIT (ptr, fp);
+
+  fflush (fp);
   if (s != NULL && *s != '\0')
     {
-      ADD (s);
-      ADD (": ");
+      WRITE_STR (s);
+      WRITE_STR (": ");
     }
 
   if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
-    ADD (error);
+    WRITE_STR (error);
 
 #ifdef __SCLE
-  ADD ((fp->_flags & __SCLE) ? "\r\n" : "\n");
+  WRITE_STR ((fp->_flags & __SCLE) ? "\r\n" : "\n");
 #else
-  ADD ("\n");
+  WRITE_STR ("\n");
 #endif
-
-  _newlib_flockfile_start (fp);
-  fflush (fp);
-  writev (fileno (fp), iov, iov_cnt);
-  _newlib_flockfile_end (fp);
 }
 
 #ifndef _REENT_ONLY
-- 
2.17.0

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

On Jul  5 09:31, Andre Vieira (lists) wrote:
> On 04/07/18 13:20, Corinna Vinschen wrote:
> > On Jul  3 19:19, Takashi Yano wrote:
> >> On Tue, 3 Jul 2018 18:22:30 +0900
> >> Takashi Yano wrote:
> >>>> I guess the simplest solution is to use the FreeBSD/OpenBSD method
> >>>> all the time.
> >>>
> >>> How about the patch attached?
> >>
> >> Fix typo in commit message.
> >>
> >> -- 
> >> Takashi Yano <takashi.yano@nifty.ne.jp>
> > 
> > Pushed.
> > 
> > 
> > Thanks,
> > Corinna
> > 
> Hi Takashi, Corinna,
> 
> This patch is breaking our arm and aarch64 bare-metal builds.  It seems
> it can't find sys/uio.h.
> 
> It seems newlib provides a couple of them:
> ./newlib/libc/machine/spu/sys/uio.h
> ./newlib/libc/sys/phoenix/sys/uio.h
> ./newlib/libc/sys/rtems/include/sys/uio.h
> ./winsup/cygwin/include/sys/uio.h
> 
> 
> Do I need to add one for arm and aarch64?  Should there be a default
> one? Suggestions?

Ouch!  I didn't realize that writev is not a required function on
bare metal, sorry.

Takashi, we need a patch to implement perror/psignal without writev, for
instance by calling write twice.  Care to send a followup patch?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Andre Vieira (lists)]

On 04/07/18 13:20, Corinna Vinschen wrote:
> On Jul  3 19:19, Takashi Yano wrote:
>> On Tue, 3 Jul 2018 18:22:30 +0900
>> Takashi Yano wrote:
>>>> I guess the simplest solution is to use the FreeBSD/OpenBSD method
>>>> all the time.
>>>
>>> How about the patch attached?
>>
>> Fix typo in commit message.
>>
>> -- 
>> Takashi Yano <takashi.yano@nifty.ne.jp>
> 
> Pushed.
> 
> 
> Thanks,
> Corinna
> 
Hi Takashi, Corinna,

This patch is breaking our arm and aarch64 bare-metal builds.  It seems
it can't find sys/uio.h.

It seems newlib provides a couple of them:
./newlib/libc/machine/spu/sys/uio.h
./newlib/libc/sys/phoenix/sys/uio.h
./newlib/libc/sys/rtems/include/sys/uio.h
./winsup/cygwin/include/sys/uio.h


Do I need to add one for arm and aarch64?  Should there be a default
one? Suggestions?

Regards,
Andre

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

On Jul  3 19:19, Takashi Yano wrote:
> On Tue, 3 Jul 2018 18:22:30 +0900
> Takashi Yano wrote:
> > > I guess the simplest solution is to use the FreeBSD/OpenBSD method
> > > all the time.
> > 
> > How about the patch attached?
> 
> Fix typo in commit message.
> 
> -- 
> Takashi Yano <takashi.yano@nifty.ne.jp>

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Brian Inglis]

On 2018-07-03 07:21, Corinna Vinschen wrote:
> On Jul  2 10:40, Brian Inglis wrote:
>> On 2018-07-02 09:46, Craig Howland wrote:
>>> On 07/02/2018 07:36 AM, Eric Blake wrote:
>>>> On 07/02/2018 05:28 AM, Corinna Vinschen wrote:
>>>>>> By the way, I have noticed that psignal() and psiginfo() also have the
>>>>>> same problem. psignal() belongs to newlib, so the same strategy can
>>>>>> be applied. However, what can we do for psiginfo()? Only the FreeBSD
>>>>>> route may be the answer...
>>>>> I guess the simplest solution is to use the FreeBSD/OpenBSD method
>>>>> all the time.
>>>> If nothing else, it at least would mean fewer variations in practice,
>>>> regardless of whether POSIX is changed to relax things (the POSIX discussion
>>>> has been started, but it may be a while before any conclusion is reached;
>>>> what's more, the C99 standard tried to address it in TC2
>>>> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_276.htm) but then withdrew
>>>> that in TC3 because the attempted resolution conflicted with the POSIX wording
>>>> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm).
>>>      To make it explicit, as looking back in the thread I don't see it stated
>>> directly:  The FreeBSD/OpenBSD method is for perror()/psignal()/psiginfo() to
>>> use write() to the stderr file number.  Supporting this as perhaps a good way to
>>> go, the discussion thread on POSIX has pointed out that "the historic [perror()]
>>> implementation calls write(2) to STDERR_FILENO."
>>>      This sounds like a good solution for newlib from the points of view of 1)
>>> staying small and uncomplicated, 2) producing behavior consistent with
>>> historical systems, 3) not changing the stream orientation.  If does have a
>>> potential drawback of possibly changing behavior since it would effectively mix
>>> stream and write output from the application level whereas now it is only
>>> stream.  On the other hand, since perror() does print a complete line, the
>>> behavior should not change if the stream is either unbuffered or line buffered,
>>> but could be different only if fully buffered.  However, the since stderr
>>> default "is not fully buffered", the number of applications affected would
>>> probably be in the minority.  (A subjective statement, but I expect it would be
>>> very hard to quantify.)
>>> [...]
>>
>> DR 322 says:
>> "Committee Discussion (for history only)
>>
>> The Committee discussed making the behavior undefined, which would allow
>> perror() to fail if the stream orientation has already been set to wide.
>>
>> The proposed TC will permit (but not require) perror to set the orientation of
>> an un-oriented stderr to narrow, and has what C calls undefined behavior if
>> stderr was previously set to wide. This permits the POSIX required behavior."
> 
> I'm not overly sympathetic with this approach.  It opens up the standard
> in a way which basically means, we allow any existing implementation to
> claim standard compliance.
> 
> The side-effect is that now applications may have to worry about the 
> orientation when usingg perror, not quite as, but similar to the
> description in http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm
> I'd rather see a better defense of the "don't change orientation".

WG14 can only document as standard what is commonly implemented that will not
get a veto from Intl. Old Crufty Compiler Co. lobbying their ISO C country rep,
who may work for them, or a government dept. using their products.

> Either way, to be on the safe side I think we should really do what
> FreeBSD/OpenBSD (and Linux, just differently) are doing.
> 
> All interested parties ok with that?  If so, I think Takashi's patch can
> go in.

Pity 64 bit was not ubiquious earlier so 32 bit wide chars became standard and
nobody got the idea that a 16 bit wide char internal format should be used for
text I/O ;^>

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

On Jul  3 19:19, Takashi Yano wrote:
> On Tue, 3 Jul 2018 18:22:30 +0900
> Takashi Yano wrote:
> > > I guess the simplest solution is to use the FreeBSD/OpenBSD method
> > > all the time.
> > 
> > How about the patch attached?
> 
> Fix typo in commit message.

LGTM.  Let's wait a bit if there's more to discuss.  If everybody
is happy with this approach, I'll push your patch.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 3558 bytes --]

On Jul  2 10:40, Brian Inglis wrote:
> On 2018-07-02 09:46, Craig Howland wrote:
> > On 07/02/2018 07:36 AM, Eric Blake wrote:
> >> On 07/02/2018 05:28 AM, Corinna Vinschen wrote:
> >>>> By the way, I have noticed that psignal() and psiginfo() also have the
> >>>> same problem. psignal() belongs to newlib, so the same strategy can
> >>>> be applied. However, what can we do for psiginfo()? Only the FreeBSD
> >>>> route may be the answer...
> >>> I guess the simplest solution is to use the FreeBSD/OpenBSD method
> >>> all the time.
> >> If nothing else, it at least would mean fewer variations in practice,
> >> regardless of whether POSIX is changed to relax things (the POSIX discussion
> >> has been started, but it may be a while before any conclusion is reached;
> >> what's more, the C99 standard tried to address it in TC2
> >> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_276.htm) but then withdrew
> >> that in TC3 because the attempted resolution conflicted with the POSIX wording
> >> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm).
> >      To make it explicit, as looking back in the thread I don't see it stated
> > directly:  The FreeBSD/OpenBSD method is for perror()/psignal()/psiginfo() to
> > use write() to the stderr file number.  Supporting this as perhaps a good way to
> > go, the discussion thread on POSIX has pointed out that "the historic [perror()]
> > implementation calls write(2) to STDERR_FILENO."
> >      This sounds like a good solution for newlib from the points of view of 1)
> > staying small and uncomplicated, 2) producing behavior consistent with
> > historical systems, 3) not changing the stream orientation.  If does have a
> > potential drawback of possibly changing behavior since it would effectively mix
> > stream and write output from the application level whereas now it is only
> > stream.  On the other hand, since perror() does print a complete line, the
> > behavior should not change if the stream is either unbuffered or line buffered,
> > but could be different only if fully buffered.  However, the since stderr
> > default "is not fully buffered", the number of applications affected would
> > probably be in the minority.  (A subjective statement, but I expect it would be
> > very hard to quantify.)
> > [...]
> 
> DR 322 says:
> "Committee Discussion (for history only)
> 
> The Committee discussed making the behavior undefined, which would allow
> perror() to fail if the stream orientation has already been set to wide.
> 
> The proposed TC will permit (but not require) perror to set the orientation of
> an un-oriented stderr to narrow, and has what C calls undefined behavior if
> stderr was previously set to wide. This permits the POSIX required behavior."

I'm not overly sympathetic with this approach.  It opens up the standard
in a way which basically means, we allow any existing implementation to
claim standard compliance.

The side-effect is that now applications may have to worry about the 
orientation when usingg perror, not quite as, but similar to the
description in http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm
I'd rather see a better defense of the "don't change orientation".

Either way, to be on the safe side I think we should really do what
FreeBSD/OpenBSD (and Linux, just differently) are doing.

All interested parties ok with that?  If so, I think Takashi's patch can
go in.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 252 bytes --]

On Tue, 3 Jul 2018 18:22:30 +0900
Takashi Yano wrote:
> > I guess the simplest solution is to use the FreeBSD/OpenBSD method
> > all the time.
> 
> How about the patch attached?

Fix typo in commit message.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-Fix-a-bug-of-perror-psignal-that-changes-the-orienta.patch --]
[-- Type: application/octet-stream, Size: 2897 bytes --]

From 80be9c95f8e48a516705b61077f8dd84e2e26878 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Tue, 3 Jul 2018 18:04:31 +0900
Subject: [PATCH] Fix a bug of perror()/psignal() that changes the orientation
 of stderr.

* perror.c: Fix the problem that perror() changes the orientation
  of stderr to byte-oriented mode if stderr is not oriented yet.
* psignal.c: Ditto.
---
 newlib/libc/signal/psignal.c | 30 +++++++++++++++++++++++++++---
 newlib/libc/stdio/perror.c   | 32 +++++++++++++++++++++++++++-----
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/newlib/libc/signal/psignal.c b/newlib/libc/signal/psignal.c
index 602714f49..9a584869d 100644
--- a/newlib/libc/signal/psignal.c
+++ b/newlib/libc/signal/psignal.c
@@ -32,13 +32,37 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <_ansi.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/uio.h>
+
+#define ADD(str) \
+{ \
+  v->iov_base = (void *)(str); \
+  v->iov_len = strlen (v->iov_base); \
+  v ++; \
+  iov_cnt ++; \
+}
 
 void
 psignal (int sig,
        const char *s)
 {
+  struct iovec iov[4];
+  struct iovec *v = iov;
+  int iov_cnt = 0;
+
   if (s != NULL && *s != '\0')
-    fprintf (stderr, "%s: %s\n", s, strsignal (sig));
-  else
-    fprintf (stderr, "%s\n", strsignal (sig));
+    {
+      ADD (s);
+      ADD (": ");
+    }
+  ADD (strsignal (sig));
+
+#ifdef __SCLE
+  ADD ((stderr->_flags & __SCLE) ? "\r\n" : "\n");
+#else
+  ADD ("\n");
+#endif
+
+  fflush (stderr);
+  writev (fileno (stderr), iov, iov_cnt);
 }
diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index d98e17e19..831c67eef 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -56,26 +56,48 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <reent.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/uio.h>
 #include "local.h"
 
+#define ADD(str) \
+{ \
+  v->iov_base = (void *)(str); \
+  v->iov_len = strlen (v->iov_base); \
+  v ++; \
+  iov_cnt ++; \
+}
+
 void
 _perror_r (struct _reent *ptr,
        const char *s)
 {
   char *error;
   int dummy;
+  struct iovec iov[4];
+  struct iovec *v = iov;
+  int iov_cnt = 0;
+  FILE *fp = _stderr_r (ptr);
 
-  _REENT_SMALL_CHECK_INIT (ptr);
+  CHECK_INIT (ptr, fp);
   if (s != NULL && *s != '\0')
     {
-      fputs (s, _stderr_r (ptr));
-      fputs (": ", _stderr_r (ptr));
+      ADD (s);
+      ADD (": ");
     }
 
   if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
-    fputs (error, _stderr_r (ptr));
+    ADD (error);
+
+#ifdef __SCLE
+  ADD ((fp->_flags & __SCLE) ? "\r\n" : "\n");
+#else
+  ADD ("\n");
+#endif
 
-  fputc ('\n', _stderr_r (ptr));
+  _newlib_flockfile_start (fp);
+  fflush (fp);
+  writev (fileno (fp), iov, iov_cnt);
+  _newlib_flockfile_end (fp);
 }
 
 #ifndef _REENT_ONLY
-- 
2.17.0

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

Hi Corinna,

On Mon, 2 Jul 2018 12:28:38 +0200
Corinna Vinschen wrote:
> Not sure if I'm missing something, but doesn't that mean the perror
> output won't use text mode even if it's requested?

It seems that the text mode only affects to outputting '\n'. So, this
is maintained in the patch. However, this is not so smart.

> I guess the simplest solution is to use the FreeBSD/OpenBSD method
> all the time.

How about the patch attached?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-Fix-a-bug-of-perror-psignal-that-changes-the-orienta.patch --]
[-- Type: application/octet-stream, Size: 2897 bytes --]

From 80be9c95f8e48a516705b61077f8dd84e2e26878 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Tue, 3 Jul 2018 18:04:31 +0900
Subject: [PATCH] Fix a bug of perror()/psignal() that changes the orientation
 of stderr.

* perror.c: Fix the problem that perror() changes the orientation
  of stderr ty byte-oriented mode if stderr is not oriented yet.
* psignal.c: Ditto.
---
 newlib/libc/signal/psignal.c | 30 +++++++++++++++++++++++++++---
 newlib/libc/stdio/perror.c   | 32 +++++++++++++++++++++++++++-----
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/newlib/libc/signal/psignal.c b/newlib/libc/signal/psignal.c
index 602714f49..9a584869d 100644
--- a/newlib/libc/signal/psignal.c
+++ b/newlib/libc/signal/psignal.c
@@ -32,13 +32,37 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <_ansi.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/uio.h>
+
+#define ADD(str) \
+{ \
+  v->iov_base = (void *)(str); \
+  v->iov_len = strlen (v->iov_base); \
+  v ++; \
+  iov_cnt ++; \
+}
 
 void
 psignal (int sig,
        const char *s)
 {
+  struct iovec iov[4];
+  struct iovec *v = iov;
+  int iov_cnt = 0;
+
   if (s != NULL && *s != '\0')
-    fprintf (stderr, "%s: %s\n", s, strsignal (sig));
-  else
-    fprintf (stderr, "%s\n", strsignal (sig));
+    {
+      ADD (s);
+      ADD (": ");
+    }
+  ADD (strsignal (sig));
+
+#ifdef __SCLE
+  ADD ((stderr->_flags & __SCLE) ? "\r\n" : "\n");
+#else
+  ADD ("\n");
+#endif
+
+  fflush (stderr);
+  writev (fileno (stderr), iov, iov_cnt);
 }
diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index d98e17e19..831c67eef 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -56,26 +56,48 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <reent.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/uio.h>
 #include "local.h"
 
+#define ADD(str) \
+{ \
+  v->iov_base = (void *)(str); \
+  v->iov_len = strlen (v->iov_base); \
+  v ++; \
+  iov_cnt ++; \
+}
+
 void
 _perror_r (struct _reent *ptr,
        const char *s)
 {
   char *error;
   int dummy;
+  struct iovec iov[4];
+  struct iovec *v = iov;
+  int iov_cnt = 0;
+  FILE *fp = _stderr_r (ptr);
 
-  _REENT_SMALL_CHECK_INIT (ptr);
+  CHECK_INIT (ptr, fp);
   if (s != NULL && *s != '\0')
     {
-      fputs (s, _stderr_r (ptr));
-      fputs (": ", _stderr_r (ptr));
+      ADD (s);
+      ADD (": ");
     }
 
   if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
-    fputs (error, _stderr_r (ptr));
+    ADD (error);
+
+#ifdef __SCLE
+  ADD ((fp->_flags & __SCLE) ? "\r\n" : "\n");
+#else
+  ADD ("\n");
+#endif
 
-  fputc ('\n', _stderr_r (ptr));
+  _newlib_flockfile_start (fp);
+  fflush (fp);
+  writev (fileno (fp), iov, iov_cnt);
+  _newlib_flockfile_end (fp);
 }
 
 #ifndef _REENT_ONLY
-- 
2.17.0

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Brian Inglis]

On 2018-07-02 09:46, Craig Howland wrote:
> On 07/02/2018 07:36 AM, Eric Blake wrote:
>> On 07/02/2018 05:28 AM, Corinna Vinschen wrote:
>>>> By the way, I have noticed that psignal() and psiginfo() also have the
>>>> same problem. psignal() belongs to newlib, so the same strategy can
>>>> be applied. However, what can we do for psiginfo()? Only the FreeBSD
>>>> route may be the answer...
>>> I guess the simplest solution is to use the FreeBSD/OpenBSD method
>>> all the time.
>> If nothing else, it at least would mean fewer variations in practice,
>> regardless of whether POSIX is changed to relax things (the POSIX discussion
>> has been started, but it may be a while before any conclusion is reached;
>> what's more, the C99 standard tried to address it in TC2
>> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_276.htm) but then withdrew
>> that in TC3 because the attempted resolution conflicted with the POSIX wording
>> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm).
>      To make it explicit, as looking back in the thread I don't see it stated
> directly:  The FreeBSD/OpenBSD method is for perror()/psignal()/psiginfo() to
> use write() to the stderr file number.  Supporting this as perhaps a good way to
> go, the discussion thread on POSIX has pointed out that "the historic [perror()]
> implementation calls write(2) to STDERR_FILENO."
>      This sounds like a good solution for newlib from the points of view of 1)
> staying small and uncomplicated, 2) producing behavior consistent with
> historical systems, 3) not changing the stream orientation.  If does have a
> potential drawback of possibly changing behavior since it would effectively mix
> stream and write output from the application level whereas now it is only
> stream.  On the other hand, since perror() does print a complete line, the
> behavior should not change if the stream is either unbuffered or line buffered,
> but could be different only if fully buffered.  However, the since stderr
> default "is not fully buffered", the number of applications affected would
> probably be in the minority.  (A subjective statement, but I expect it would be
> very hard to quantify.)
>      It is a bad solution from the point of view that the C standard (C11 N1570
> 7.21.10.4#2) says that perror() "writes a sequence of characters to the standard
> error stream".  That is, using write does not write to the stream, itself, as
> required.  So the question becomes, is this a real problem, or only a
> theoretical problem? Since write ultimately is called for writes to the stream,
> how real of a problem is it?  As already noted, there is one settings case
> (fully buffered) in which actual behavior could be different.
>      I think that one way of quantifying the general problem is that we have a
> choice between 3 options.   1) violate a POSIX CX extension (present state), 2) 
> remain strictly C and POSIX CX compliant (which the present thread has
> characterized as making the code a lot more complicated), or 3) violate the C
> standard (albeit in a way that can be argued to be meaningless).
>      (To be very picky, the preceding evaluations as to how settings might
> affect output are based on using newlib as it stands. Obviously, if anyone has
> partially "hacked" the stream method for efficiency purposes, things would
> differ more.  But this is an inherent danger in such edits, and should be
> understood as a risk by anyone who does them.  (I have done such in the past in
> highly-constrained systems, but I knew I might be stuck at a specific newlib
> version when I did so.))

DR 322 says:
"Committee Discussion (for history only)

The Committee discussed making the behavior undefined, which would allow
perror() to fail if the stream orientation has already been set to wide.

The proposed TC will permit (but not require) perror to set the orientation of
an un-oriented stderr to narrow, and has what C calls undefined behavior if
stderr was previously set to wide. This permits the POSIX required behavior."

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Craig Howland]

On 07/02/2018 07:36 AM, Eric Blake wrote:
> On 07/02/2018 05:28 AM, Corinna Vinschen wrote:
>
>>> By the way, I have noticed that psignal() and psiginfo() also have the
>>> same problem. psignal() belongs to newlib, so the same strategy can
>>> be applied. However, what can we do for psiginfo()? Only the FreeBSD
>>> route may be the answer...
>>
>> I guess the simplest solution is to use the FreeBSD/OpenBSD method
>> all the time.
>
> If nothing else, it at least would mean fewer variations in practice, 
> regardless of whether POSIX is changed to relax things (the POSIX discussion 
> has been started, but it may be a while before any conclusion is reached; 
> what's more, the C99 standard tried to address it in TC2 
> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_276.htm) but then withdrew 
> that in TC3 because the attempted resolution conflicted with the POSIX wording 
> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm).
>
      To make it explicit, as looking back in the thread I don't see it stated 
directly:  The FreeBSD/OpenBSD method is for perror()/psignal()/psiginfo() to 
use write() to the stderr file number.  Supporting this as perhaps a good way to 
go, the discussion thread on POSIX has pointed out that "the historic [perror()] 
implementation calls write(2) to STDERR_FILENO."
      This sounds like a good solution for newlib from the points of view of 1) 
staying small and uncomplicated, 2) producing behavior consistent with 
historical systems, 3) not changing the stream orientation.  If does have a 
potential drawback of possibly changing behavior since it would effectively mix 
stream and write output from the application level whereas now it is only 
stream.  On the other hand, since perror() does print a complete line, the 
behavior should not change if the stream is either unbuffered or line buffered, 
but could be different only if fully buffered.  However, the since stderr 
default "is not fully buffered", the number of applications affected would 
probably be in the minority.  (A subjective statement, but I expect it would be 
very hard to quantify.)
      It is a bad solution from the point of view that the C standard (C11 N1570 
7.21.10.4#2) says that perror() "writes a sequence of characters to the standard 
error stream".  That is, using write does not write to the stream, itself, as 
required.  So the question becomes, is this a real problem, or only a 
theoretical problem? Since write ultimately is called for writes to the stream, 
how real of a problem is it?  As already noted, there is one settings case 
(fully buffered) in which actual behavior could be different.
      I think that one way of quantifying the general problem is that we have a 
choice between 3 options.   1) violate a POSIX CX extension (present state), 2)  
remain strictly C and POSIX CX compliant (which the present thread has 
characterized as making the code a lot more complicated), or 3) violate the C 
standard (albeit in a way that can be argued to be meaningless).
      (To be very picky, the preceding evaluations as to how settings might 
affect output are based on using newlib as it stands. Obviously, if anyone has 
partially "hacked" the stream method for efficiency purposes, things would 
differ more.  But this is an inherent danger in such edits, and should be 
understood as a risk by anyone who does them.  (I have done such in the past in 
highly-constrained systems, but I knew I might be stuck at a specific newlib 
version when I did so.))
                 Craig

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

Hi Eric,

On Jul  2 06:36, Eric Blake wrote:
> On 07/02/2018 05:28 AM, Corinna Vinschen wrote:
> 
> > > By the way, I have noticed that psignal() and psiginfo() also have the
> > > same problem. psignal() belongs to newlib, so the same strategy can
> > > be applied. However, what can we do for psiginfo()? Only the FreeBSD
> > > route may be the answer...
> > 
> > I guess the simplest solution is to use the FreeBSD/OpenBSD method
> > all the time.
> 
> If nothing else, it at least would mean fewer variations in practice,
> regardless of whether POSIX is changed to relax things (the POSIX discussion
> has been started, but it may be a while before any conclusion is reached;
> what's more, the C99 standard tried to address it in TC2
> (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_276.htm) but then
> withdrew that in TC3 because the attempted resolution conflicted with the
> POSIX wording (http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm).

Thanks for keeping us informed!


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Eric Blake]

On 07/02/2018 05:28 AM, Corinna Vinschen wrote:

>> By the way, I have noticed that psignal() and psiginfo() also have the
>> same problem. psignal() belongs to newlib, so the same strategy can
>> be applied. However, what can we do for psiginfo()? Only the FreeBSD
>> route may be the answer...
> 
> I guess the simplest solution is to use the FreeBSD/OpenBSD method
> all the time.

If nothing else, it at least would mean fewer variations in practice, 
regardless of whether POSIX is changed to relax things (the POSIX 
discussion has been started, but it may be a while before any conclusion 
is reached; what's more, the C99 standard tried to address it in TC2 
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_276.htm) but then 
withdrew that in TC3 because the attempted resolution conflicted with 
the POSIX wording 
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_322.htm).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]

On Jun 29 21:34, Takashi Yano wrote:
> I am not in a position to comment whether POSIX is wrong or not, so
> nothing about that.

Well, either we all are in this position or nobody of us ;)

> On Thu, 28 Jun 2018 20:31:57 +0200
> Corinna Vinschen wrote:
> > > I tried this but it failed because __sputc_r() called from _fputs_r also
> > > sets orientation. So I have borrowed the codes from __swbuf_r() in wbuf.c
> > > so that the strings are pushed directly into the buffer.
> > 
> > I did.  Thanks for implementing this, but... uhm... I'm not really
> > thrilled.  So much extra code for such a simple thing as perror...?
> > 
> > On second thought I wonder if we shouldn't just go the FreeBSD route.
> 
> Maybe. But I have another possibility. What about this one?

Not sure if I'm missing something, but doesn't that mean the perror
output won't use text mode even if it's requested?

> By the way, I have noticed that psignal() and psiginfo() also have the
> same problem. psignal() belongs to newlib, so the same strategy can
> be applied. However, what can we do for psiginfo()? Only the FreeBSD
> route may be the answer...

I guess the simplest solution is to use the FreeBSD/OpenBSD method
all the time.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

I am not in a position to comment whether POSIX is wrong or not, so
nothing about that.

On Thu, 28 Jun 2018 20:31:57 +0200
Corinna Vinschen wrote:
> > I tried this but it failed because __sputc_r() called from _fputs_r also
> > sets orientation. So I have borrowed the codes from __swbuf_r() in wbuf.c
> > so that the strings are pushed directly into the buffer.
> 
> I did.  Thanks for implementing this, but... uhm... I'm not really
> thrilled.  So much extra code for such a simple thing as perror...?
> 
> On second thought I wonder if we shouldn't just go the FreeBSD route.

Maybe. But I have another possibility. What about this one?

By the way, I have noticed that psignal() and psiginfo() also have the
same problem. psignal() belongs to newlib, so the same strategy can
be applied. However, what can we do for psiginfo()? Only the FreeBSD
route may be the answer...

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-Fix-a-bug-of-perror-which-changes-the-orientation-of.patch --]
[-- Type: application/octet-stream, Size: 2147 bytes --]

From 05c640812531f758d42502f4c144ee09f82406ee Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Fri, 29 Jun 2018 21:19:50 +0900
Subject: [PATCH] Fix a bug of perror() which changes the orientation of
 stderr.

* perror.c: Fix the problem that perror() changes the orientation
  of stderr to byte-oriented mode if stderr is not oriented yet.
---
 newlib/libc/stdio/perror.c | 44 ++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index d98e17e19..dd277370a 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -56,26 +56,58 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <reent.h>
 #include <stdio.h>
 #include <string.h>
+#include "fvwrite.h"
 #include "local.h"
 
+#define ADD(str) \
+{ \
+  v->iov_base = (str); \
+  v->iov_len = strlen (v->iov_base); \
+  uio.uio_iovcnt++; \
+  uio.uio_resid += v->iov_len; \
+  v++; \
+}
+
 void
 _perror_r (struct _reent *ptr,
        const char *s)
 {
   char *error;
   int dummy;
+  struct __suio uio;
+  struct __siov iov[4];
+  struct __siov *v;
+  short flag_scle = 0;
+  FILE *fp;
+
+  fp = _stderr_r (ptr);
 
-  _REENT_SMALL_CHECK_INIT (ptr);
+  CHECK_INIT (ptr, fp);
+
+  uio.uio_iov = v = iov;
+  uio.uio_iovcnt = 0;
+  uio.uio_resid = 0;
   if (s != NULL && *s != '\0')
     {
-      fputs (s, _stderr_r (ptr));
-      fputs (": ", _stderr_r (ptr));
+      ADD (s);
+      ADD (": ");
     }
 
   if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
-    fputs (error, _stderr_r (ptr));
-
-  fputc ('\n', _stderr_r (ptr));
+    ADD (error);
+#ifdef __SCLE
+  flag_scle = fp->_flags & __SCLE;
+#endif
+  ADD (flag_scle ? "\r\n" : "\n");
+
+  _newlib_flockfile_start (fp);
+  /* The orientation is set in __sfvwrite_r() if __SCLE is set.
+     Therefore unset __SCLE temporarily, and restore it after
+     calling __sfvwrite_r(). */
+  fp->_flags &= ~flag_scle;
+  __sfvwrite_r (ptr, fp, &uio);
+  fp->_flags |= flag_scle;
+  _newlib_flockfile_end (fp);
 }
 
 #ifndef _REENT_ONLY
-- 
2.17.0

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

On Jun 28 12:28, Craig Howland wrote:
> On 06/27/2018 08:55 AM, Corinna Vinschen wrote:
> > ...
> > 
> > 
> > On Jun 27 20:01, Takashi Yano wrote:
> > > POSIX states:
> > > The perror() function shall not change the orientation of the standard
> > > error stream.
> > > 
> > > However, cygwin perror() function changes the orientation of stderr to
> > > byte-oriented mode if stderr is not oriented yet.
> I suggest that POSIX is in error.  The POSIX statement about not changing
> the orientation is an extension to the C standard (CX, to be precise). 
> POSIX is always careful to defer to the C standard, which I think does
> indirectly specify that perror() is byte-oriented.  The C standard actually
> does not directly talk about the orientation of perror().  However, it
> directly defines (quoting from the N1570 C11 draft):
> [...]
> Therefore, the newlib perror() behavior is correct and should not be
> changed.  It definitely is a mess and there really ought to be a perrorw()
> function.

Interesting discussion.

Under the assumption the standard isn't quite clear on this, we have 
OpenBSD, FreeBSD and Linux interpreting the standard as "don't change
orientiation from perror".  NetBSD doesn't care, just like Newlib.

As far as I can see, the NetBSD and the Newlib code are older than
the interpretation of the other three.  Does the new interpretation
set a precedent?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Craig Howland]

On 06/28/2018 01:54 PM, Brian Inglis wrote:
> On 2018-06-28 10:28, Craig Howland wrote:
>> On 06/27/2018 08:55 AM, Corinna Vinschen wrote:
>>> On Jun 27 20:01, Takashi Yano wrote:
>>>> POSIX states:
>>>> The perror() function shall not change the orientation of the standard
>>>> error stream.
>>>> However, cygwin perror() function changes the orientation of stderr to
>>>> byte-oriented mode if stderr is not oriented yet.
>> I suggest that POSIX is in error.  ...
> INCITS/ISO/IEC 9899-2011[2012] 7.21 Input/output <stdio.h> 7.21.1 Introduction
> #5 is exactly the same.
>
> However 7.21.3 Files #13 states:
> "In some cases, some of the byte input/output functions also perform conversions
> between multibyte characters and wide characters. These conversions also occur
> as if by calls to the mbrtowc and wcrtomb functions."
>
> This seems to allow for input/output functions not listed in 7.21.1#5 to adapt
> to the stream orientation by using the stream's character input/output
> conversion functions, which could be as above, or a nop.
I had missed that (being under 7.21.3 Files heading rather than 7.21.2 
Streams).  But how are you to know which ones "some" is? There does not seem to 
be a direct list of ones that should be able to do this.  However, the fprintf() 
description of the s format specifier says "If an l length modifier is present, 
the argument shall be a pointer to the initial element of an array of wchar_t 
type. Wide characters from the array are converted to multibyte characters (each 
as if by a call to the wcrtomb function, with the conversion state described by 
an mbstate_t object initialized to zero before the first wide character is 
converted) up to and including a terminating null wide character. The resulting 
multibyte characters are written up to (but not including) the terminating null 
character (byte)."  So fprintf() directly calls for the stated 7.21.1#5 
ability.  Similarly, fscanf() also calls for reverse ability for %ls.  On the 
other hand, perror() does not.  So a reasonable inference is that 7.21.1#5 only 
applies to fprintf() and fscanf() and their derivatives because they directly 
call for the ability, directly mentioning wide to multbyte (and vice versa), 
while the other functions in stdio.h do not (at least, not that I found when 
searching for "multibyte").  While it is an inference that it should not apply 
to others, the inference is supported by the statement that it is "SOME of the 
byte input/output functions": if it is not limited to the ones which 
specifically call for it, then the logical conclusion would be that it should be 
all of them. (It sure would be better if the standard were more specific.)
      (Note:  both fprintf() and fscanf() call for their formats to be 
multibyte.  (E.g. (.21.6.1#3 says "The format shall be a multibyte character 
sequence, beginning and ending in its initial shift state. The format is 
composed of zero or more directives: ordinary multibyte characters (not %), 
which are copied unchanged to the output stream; ...".)  But this spec on the 
formats is not the 7.21.1#5 ability, as it is multibyte passed through to 
byte--it is not conversion of wide to multibyte or vice versa.)
>
>> Please note that perror() is not listed.  ...
>>
>> (Granted, I should probably submit these arguments to POSIX for evaluation, but
>> I don't know how.  Perhaps Eric Blake might be able to help with this,
>> potentially with an off-list discussion.)
> Joseph Myers has some experience submitting C DRs to JTC1/SC22/WG14, who should
> be responsible for any clarification required. Discussions are better on-list.
>
(I didn't mean to discuss the topic off list, I meant possibly discussing the 
mechanics of how this might be done.  That is off the direct topic, but point 
taken, even that might be appropriate for the list.)
By the way, Eric Blake did forward my initial email to the Austin Group to ask 
for an opinion on my opinion (thank you).  I'm sure he'll keep the newlib list 
informed of what that produces.
Craig

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

Hi Takashi,

On Jun 28 20:14, Takashi Yano wrote:
> Hi Corinna,
> 
> On Wed, 27 Jun 2018 14:55:03 +0200
> Corinna Vinschen wrote:
> > again, please send patches related to newlib to the newlib mailing list.
> > Newlib patches affect more targets than just Cygwin.  I redirected this
> > mail to the newlib list and attached your original attachments.  Thank
> > you.
> 
> Next time I will. Sorry for my inadequate understanding about newlib.

No worries.  When coming from the Cygwin side it's not immediately
obvious.

> > I'm not sure exactly.  It may be nice to keep the writes buffered
> > if the original stderr stream is buffered as well.
> > 
> > What about duplicating the non-_FVWRITE_IN_STREAMIO part of _fputs_r,
> > just without calling ORIENT?
> 
> Thank you for your suggestion.
> 
> I tried this but it failed because __sputc_r() called from _fputs_r also
> sets orientation. So I have borrowed the codes from __swbuf_r() in wbuf.c
> so that the strings are pushed directly into the buffer.
> 
> Could you please have a look?

I did.  Thanks for implementing this, but... uhm... I'm not really
thrilled.  So much extra code for such a simple thing as perror...?

On second thought I wonder if we shouldn't just go the FreeBSD route.

What's your stance?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Brian Inglis]

On 2018-06-28 10:28, Craig Howland wrote:
> On 06/27/2018 08:55 AM, Corinna Vinschen wrote:
>> On Jun 27 20:01, Takashi Yano wrote:
>>> POSIX states:
>>> The perror() function shall not change the orientation of the standard
>>> error stream.
>>> However, cygwin perror() function changes the orientation of stderr to
>>> byte-oriented mode if stderr is not oriented yet.
> I suggest that POSIX is in error.  The POSIX statement about not changing the
> orientation is an extension to the C standard (CX, to be precise).  POSIX is
> always careful to defer to the C standard, which I think does indirectly specify
> that perror() is byte-oriented.  The C standard actually does not directly talk
> about the orientation of perror().  However, it directly defines (quoting from
> the N1570 C11 draft):
> 
> "The input/output functions are given the following collective terms:
> — The wide character input functions — those functions described in 7.29 that
> perform input into wide characters and wide strings: fgetwc, fgetws, getwc,
> getwchar, fwscanf, wscanf, vfwscanf, and vwscanf.
> — The wide character output functions — those functions described in 7.29 that
> perform output from wide characters and wide strings: fputwc, fputws, putwc,
> putwchar, fwprintf, wprintf, vfwprintf, and vwprintf.
> — The wide character input/output functions — the union of the ungetwc function,
> the wide character input functions, and the wide character output functions.
> — The byte input/output functions — those functions described in this subclause
> that perform input/output: fgetc, fgets, fprintf, fputc, fputs, fread, fscanf,
> fwrite, getc, getchar, printf, putc, putchar, puts, scanf, ungetc, vfprintf,
> vfscanf, vprintf, and vscanf."

INCITS/ISO/IEC 9899-2011[2012] 7.21 Input/output <stdio.h> 7.21.1 Introduction
#5 is exactly the same.

However 7.21.3 Files #13 states:
"In some cases, some of the byte input/output functions also perform conversions
between multibyte characters and wide characters. These conversions also occur
as if by calls to the mbrtowc and wcrtomb functions."

This seems to allow for input/output functions not listed in 7.21.1#5 to adapt
to the stream orientation by using the stream's character input/output
conversion functions, which could be as above, or a nop.

> Please note that perror() is not listed.  While this could be interpreted to
> mean it can be both, the proper way for that have to been done would be for it
> to appear in both lists--which it does not.  However, perror() is defined in the
> same stdio.h subclause (i.e. 7.21) as all of the byte functions, against the
> wide-character functions in wchar.h (7.29).  So even though the C standard is
> sloppy and does not directly have perror() in the enumerated list, it is
> included by the general statement about the subclause. However, you could argue
> that it was purposely left out, which is why they bothered to list the others. 
> Against this are the definition or perror(), itself, and that they really should
> have listed perror() as an exception if it was so intended, and (as
> already-mentioned) perror() should be in both lists if it is to be dual-oriented.
> 
> Here is the argument based on the perror() definition:
> 
> "void perror(const char *s);
> ...
> It writes a sequence of characters to the standard error stream thus: first (if s
> is not a null pointer and the character pointed to by s is not the null
> character), the string pointed to by s followed by a colon (:) and a space; then
> an appropriate error message string followed by a new-line character."
> 
> Things to note:
> 1)  It is a regular character pointer, not a wide character pointer.  Those
> characters, if supplied, are written.  (It says nothing about converting them to
> wide if need be, it says "the string pointed to by s".)
> 2)  "error message string".  It does not say 'or wide-character error message
> string if needed'.
> 3)  Followed by a "new-line character".  It does not say "new-line wide
> character", which is used throughout the wchar.h section (7.29).
> 
> So there is definitely a weakness in the C standard, but I think it is clear
> that perror() is a byte output function.  If the user wants to print to a
> wide-character stream, the only pure way to do it would be to turn strerror()
> (used by perror()) output into a wide-character string.  POSIX noted this
> weakness, but fixed it with a bad extension, rather than classifying perror() as
> byte--which is clearly is.
> 
> Therefore, the newlib perror() behavior is correct and should not be changed. 
> It definitely is a mess and there really ought to be a perrorw() function.
> 
> (Granted, I should probably submit these arguments to POSIX for evaluation, but
> I don't know how.  Perhaps Eric Blake might be able to help with this,
> potentially with an off-list discussion.)

Joseph Myers has some experience submitting C DRs to JTC1/SC22/WG14, who should
be responsible for any clarification required. Discussions are better on-list.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Craig Howland]

On 06/27/2018 08:55 AM, Corinna Vinschen wrote:
> ...
>
>
> On Jun 27 20:01, Takashi Yano wrote:
>> POSIX states:
>> The perror() function shall not change the orientation of the standard
>> error stream.
>>
>> However, cygwin perror() function changes the orientation of stderr to
>> byte-oriented mode if stderr is not oriented yet.
I suggest that POSIX is in error.  The POSIX statement about not changing the 
orientation is an extension to the C standard (CX, to be precise).  POSIX is 
always careful to defer to the C standard, which I think does indirectly specify 
that perror() is byte-oriented.  The C standard actually does not directly talk 
about the orientation of perror().  However, it directly defines (quoting from 
the N1570 C11 draft):

"The input/output functions are given the following collective terms:
— The wide character input functions — those functions described in 7.29 that 
perform input into wide characters and wide strings: fgetwc, fgetws, getwc, 
getwchar, fwscanf, wscanf, vfwscanf, and vwscanf.
— The wide character output functions — those functions described in 7.29 that 
perform output from wide characters and wide strings: fputwc, fputws, putwc, 
putwchar, fwprintf, wprintf, vfwprintf, and vwprintf.
— The wide character input/output functions — the union of the ungetwc function, 
the wide character input functions, and the wide character output functions.
— The byte input/output functions — those functions described in this subclause 
that perform input/output: fgetc, fgets, fprintf, fputc, fputs, fread, fscanf, 
fwrite, getc, getchar, printf, putc, putchar, puts, scanf, ungetc, vfprintf, 
vfscanf, vprintf, and vscanf."

Please note that perror() is not listed.  While this could be interpreted to 
mean it can be both, the proper way for that have to been done would be for it 
to appear in both lists--which it does not.  However, perror() is defined in the 
same stdio.h subclause (i.e. 7.21) as all of the byte functions, against the 
wide-character functions in wchar.h (7.29).  So even though the C standard is 
sloppy and does not directly have perror() in the enumerated list, it is 
included by the general statement about the subclause. However, you could argue 
that it was purposely left out, which is why they bothered to list the others.  
Against this are the definition or perror(), itself, and that they really should 
have listed perror() as an exception if it was so intended, and (as 
already-mentioned) perror() should be in both lists if it is to be dual-oriented.

Here is the argument based on the perror() definition:

"void perror(const char *s);
...
It writes a sequence of characters to the standard error stream thus: first (if s 
is not a null pointer and the character pointed to by s is not the null 
character), the string pointed to by s followed by a colon (:) and a space; then 
an appropriate error message string followed by a new-line character."

Things to note:
1)  It is a regular character pointer, not a wide character pointer.  Those 
characters, if supplied, are written.  (It says nothing about converting them to 
wide if need be, it says "the string pointed to by s".)
2)  "error message string".  It does not say 'or wide-character error message 
string if needed'.
3)  Followed by a "new-line character".  It does not say "new-line wide 
character", which is used throughout the wchar.h section (7.29).

So there is definitely a weakness in the C standard, but I think it is clear 
that perror() is a byte output function.  If the user wants to print to a 
wide-character stream, the only pure way to do it would be to turn strerror() 
(used by perror()) output into a wide-character string.  POSIX noted this 
weakness, but fixed it with a bad extension, rather than classifying perror() as 
byte--which is clearly is.

Therefore, the newlib perror() behavior is correct and should not be changed.  
It definitely is a mess and there really ought to be a perrorw() function.

(Granted, I should probably submit these arguments to POSIX for evaluation, but 
I don't know how.  Perhaps Eric Blake might be able to help with this, 
potentially with an off-list discussion.)

Craig

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 913 bytes --]

Hi Corinna,

On Wed, 27 Jun 2018 14:55:03 +0200
Corinna Vinschen wrote:
> again, please send patches related to newlib to the newlib mailing list.
> Newlib patches affect more targets than just Cygwin.  I redirected this
> mail to the newlib list and attached your original attachments.  Thank
> you.

Next time I will. Sorry for my inadequate understanding about newlib.

> I'm not sure exactly.  It may be nice to keep the writes buffered
> if the original stderr stream is buffered as well.
> 
> What about duplicating the non-_FVWRITE_IN_STREAMIO part of _fputs_r,
> just without calling ORIENT?

Thank you for your suggestion.

I tried this but it failed because __sputc_r() called from _fputs_r also
sets orientation. So I have borrowed the codes from __swbuf_r() in wbuf.c
so that the strings are pushed directly into the buffer.

Could you please have a look?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-Fix-a-bug-of-perror-which-changes-the-orientation-of.patch --]
[-- Type: application/octet-stream, Size: 2374 bytes --]

From a0932c39db0ae2a199b5238c0f65cae5e69e016c Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Thu, 28 Jun 2018 19:46:21 +0900
Subject: [PATCH] Fix a bug of perror() which changes the orientation of
 stderr.

* perror.c: Fix the problem that perror() changes the orientation
  of stderr to byte-oriented mode if stderr is not oriented yet.
---
 newlib/libc/stdio/perror.c | 54 +++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index d98e17e19..d3a3413c7 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -56,8 +56,54 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <reent.h>
 #include <stdio.h>
 #include <string.h>
+#include "fvwrite.h"
 #include "local.h"
 
+/* Use static int _fputs_r_loc() instead of fputs()/fputc()
+   to prevent the orientation from being set. */
+static int
+_fputs_r_loc (struct _reent * ptr,
+       char const *__restrict s,
+       FILE *__restrict fp)
+{
+  register int n;
+  const char *p = s;
+
+  /* Ensure stdio has been initialized.  */
+  CHECK_INIT(ptr, fp);
+
+  _newlib_flockfile_start (fp);
+
+  if (cantwrite (ptr, fp))
+    goto error;
+
+  n = fp->_p - fp->_bf._base;
+  if (n >= fp->_bf._size)
+    {
+      if (_fflush_r (ptr, fp))
+	goto error;
+      n = 0;
+    }
+  while (*p)
+    {
+      fp->_w--;
+      *fp->_p++ = *p;
+      if (++n == fp->_bf._size || (fp->_flags & __SLBF && *p == '\n'))
+	{
+	  if (_fflush_r (ptr, fp))
+	    goto error;
+	  n = 0;
+	}
+      p++;
+    }
+  _newlib_flockfile_exit (fp);
+  return 0;
+
+error:
+  _newlib_flockfile_end (fp);
+  return EOF;
+}
+
 void
 _perror_r (struct _reent *ptr,
        const char *s)
@@ -68,14 +114,14 @@ _perror_r (struct _reent *ptr,
   _REENT_SMALL_CHECK_INIT (ptr);
   if (s != NULL && *s != '\0')
     {
-      fputs (s, _stderr_r (ptr));
-      fputs (": ", _stderr_r (ptr));
+      _fputs_r_loc (ptr, s, _stderr_r (ptr));
+      _fputs_r_loc (ptr, ": ", _stderr_r (ptr));
     }
 
   if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
-    fputs (error, _stderr_r (ptr));
+    _fputs_r_loc (ptr, error, _stderr_r (ptr));
 
-  fputc ('\n', _stderr_r (ptr));
+  _fputs_r_loc (ptr, "\n", _stderr_r (ptr));
 }
 
 #ifndef _REENT_ONLY
-- 
2.17.0

Re: perror() changes the orientation of stderr to byte-oriented mode if stderr is not oriented yet.

[Corinna Vinschen]

[-- Attachment #1.1: Type: text/plain, Size: 1540 bytes --]

Hi Takashi,



again, please send patches related to newlib to the newlib mailing list.
Newlib patches affect more targets than just Cygwin.  I redirected this
mail to the newlib list and attached your original attachments.  Thank
you.



On Jun 27 20:01, Takashi Yano wrote:
> POSIX states:
> The perror() function shall not change the orientation of the standard
> error stream.
> 
> However, cygwin perror() function changes the orientation of stderr to
> byte-oriented mode if stderr is not oriented yet.

That's newlib's perror actually.

> [...]
> I have made a patch to solve this problem, attached. However, I am not
> sure that calling _write_r() here is correct manner. I will appreciate
> if anyone familiar with libc code comment or make suggestions.

I'm not sure exactly.  It may be nice to keep the writes buffered
if the original stderr stream is buffered as well.

What about duplicating the non-_FVWRITE_IN_STREAMIO part of _fputs_r,
just without calling ORIENT?

Another solution might be what glibc does; if the stream has no
orientation yet, it duplicates the stderr FILE handle and uses that to
print the string.

Checking FreeBSD, it seems it actually calls writev, without actually
checking if the entire string has been written, see
https://github.com/freebsd/freebsd/blob/master/lib/libc/stdio/perror.c


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #1.2: 0001-Fix-a-bug-of-perror-which-changes-the-orientation-of.patch --]
[-- Type: text/plain, Size: 1752 bytes --]

From cf56cf0eef8d48ca543aac96d57fdfebe3e8767a Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Wed, 27 Jun 2018 19:26:15 +0900
Subject: [PATCH] Fix a bug of perror() which changes the orientation of
 stderr.

* perror.c: Fix the problem that perror() changes the orientation
  of stderr to byte-oriented mode if stderr is not oriented yet.
---
 newlib/libc/stdio/perror.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/newlib/libc/stdio/perror.c b/newlib/libc/stdio/perror.c
index d98e17e19..3328425da 100644
--- a/newlib/libc/stdio/perror.c
+++ b/newlib/libc/stdio/perror.c
@@ -58,6 +58,23 @@ Supporting OS subroutines required: <<close>>, <<fstat>>, <<isatty>>,
 #include <string.h>
 #include "local.h"
 
+static int
+write_err (struct _reent *ptr, const char *s)
+{
+  size_t len = strlen (s);
+  size_t len_out = 0;
+  int fd_err = fileno (_stderr_r (ptr));
+  while (len_out != len)
+    {
+      _ssize_t len1 = _write_r (ptr, fd_err, s+len_out, len-len_out);
+      if (len1 > 0)
+	len_out += len1;
+      else
+	return -1;
+    }
+  return 0;
+}
+
 void
 _perror_r (struct _reent *ptr,
        const char *s)
@@ -68,14 +85,14 @@ _perror_r (struct _reent *ptr,
   _REENT_SMALL_CHECK_INIT (ptr);
   if (s != NULL && *s != '\0')
     {
-      fputs (s, _stderr_r (ptr));
-      fputs (": ", _stderr_r (ptr));
+      write_err (ptr, s);
+      write_err (ptr, ": ");
     }
 
   if ((error = _strerror_r (ptr, ptr->_errno, 1, &dummy)) != NULL)
-    fputs (error, _stderr_r (ptr));
+    write_err (ptr, error);
 
-  fputc ('\n', _stderr_r (ptr));
+  write_err (ptr, "\n");
 }
 
 #ifndef _REENT_ONLY
-- 
2.17.0


[-- Attachment #1.3: chkperror.c --]
[-- Type: text/plain, Size: 352 bytes --]

#include <stdio.h>
#include <wchar.h>
#include <locale.h>
#include <errno.h>

int main()
{
	setlocale(LC_CTYPE, "de_DE.UTF-8");
	errno = EINVAL;
	printf("%d\n", fwide(stderr, 0));
	perror("äöü");
	printf("%d\n", fwide(stderr, 0));
	fwprintf(stderr, L"äöü\n");
	printf("%d\n", fwide(stderr, 0));
	perror("äöü");
	return 0;
}


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]