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

[Corinna Vinschen]

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.

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

[Takashi Yano]

Hi Corinna,

On Wed, 27 Jun 2018 14:55:03 +0200

Post by Corinna Vinschen
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.

Post by Corinna Vinschen
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?

[Corinna Vinschen]

Hi Takashi,

Post by Takashi Yano
Hi Corinna,
On Wed, 27 Jun 2018 14:55:03 +0200

Post by Corinna Vinschen
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.

Post by Takashi Yano

Post by Corinna Vinschen
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

[Takashi Yano]

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

Post by Corinna Vinschen

Post by Takashi Yano
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...

[Corinna Vinschen]

Post by Takashi Yano
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 ;)

Post by Takashi Yano
On Thu, 28 Jun 2018 20:31:57 +0200

Post by Corinna Vinschen

Post by Takashi Yano
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?

Post by Takashi Yano
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

[Eric Blake]

Post by Corinna Vinschen

Post by Takashi Yano
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).

[Corinna Vinschen]

Hi Eric,

Post by Eric Blake

Post by Corinna Vinschen

Post by Takashi Yano
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

[Craig Howland]

Post by Eric Blake

Post by Corinna Vinschen

Post by Takashi Yano
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

[Brian Inglis]

Post by Craig Howland

Post by Eric Blake

Post by Corinna Vinschen

Post by Takashi Yano
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."

[Corinna Vinschen]

Post by Brian Inglis

Post by Eric Blake

Post by Corinna Vinschen

Post by Takashi Yano
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.)
[...]

"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

[Brian Inglis]

Post by Corinna Vinschen

Post by Brian Inglis

Post by Craig Howland

Post by Eric Blake

Post by Corinna Vinschen

Post by Takashi Yano
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.)
[...]

"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.

Post by Corinna Vinschen
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 ;^>

[Takashi Yano]

Hi Corinna,

On Mon, 2 Jul 2018 12:28:38 +0200

Post by Corinna Vinschen
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.

Post by Corinna Vinschen
I guess the simplest solution is to use the FreeBSD/OpenBSD method
all the time.

How about the patch attached?

[Takashi Yano]

On Tue, 3 Jul 2018 18:22:30 +0900

Post by Takashi Yano

Post by Corinna Vinschen
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.

[Corinna Vinschen]

Post by Takashi Yano
On Tue, 3 Jul 2018 18:22:30 +0900

Post by Takashi Yano

Post by Corinna Vinschen
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]

Post by Takashi Yano
On Tue, 3 Jul 2018 18:22:30 +0900

Post by Takashi Yano

Post by Corinna Vinschen
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.
--

Pushed.


Thanks,
Corinna

[Andre Vieira (lists)]

Post by Corinna Vinschen

Post by Takashi Yano
On Tue, 3 Jul 2018 18:22:30 +0900

Post by Takashi Yano

Post by Corinna Vinschen
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.
--

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

[Corinna Vinschen]

Post by Andre Vieira (lists)

Post by Corinna Vinschen

Post by Takashi Yano
On Tue, 3 Jul 2018 18:22:30 +0900

Post by Takashi Yano

Post by Corinna Vinschen
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.
--

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.
./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

[Takashi Yano]

Hi Corinna,

On Thu, 5 Jul 2018 11:08:51 +0200

Post by Corinna Vinschen
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?

[Corinna Vinschen]

Post by Takashi Yano
Hi Corinna,
On Thu, 5 Jul 2018 11:08:51 +0200

Post by Corinna Vinschen
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;

Post by Takashi Yano
+#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; \

Post by Takashi Yano
[...]
+#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.

Post by Takashi Yano
+ 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

[Takashi Yano]

Hi Corinna,

Thank you for checking.

On Thu, 5 Jul 2018 13:19:21 +0200

Post by Corinna Vinschen
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
stderr->_flags &= ~__SOFF;

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

[Corinna Vinschen]

Post by Takashi Yano
Hi Corinna,
Thank you for checking.
On Thu, 5 Jul 2018 13:19:21 +0200

Post by Corinna Vinschen
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
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

[Takashi Yano]

On Thu, 5 Jul 2018 14:49:00 +0200

Post by Corinna Vinschen
...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.

...

Post by Corinna Vinschen
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.

[Jeff Johnston]

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

-- Jeff J.

Post by Takashi Yano
On Thu, 5 Jul 2018 14:49:00 +0200

Post by Corinna Vinschen
...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.

...

Post by Corinna Vinschen
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.
--

[Corinna Vinschen]

Post by Jeff Johnston
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

Post by Jeff Johnston
-- Jeff J.

Post by Takashi Yano
On Thu, 5 Jul 2018 14:49:00 +0200

Post by Corinna Vinschen
...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.

...

Post by Corinna Vinschen
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.
--

[Craig Howland]

Post by Corinna Vinschen

Post by Takashi Yano
Hi Corinna,
Thank you for checking.
On Thu, 5 Jul 2018 13:19:21 +0200

Post by Corinna Vinschen
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
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

[Craig Howland]

...

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

[Brian Inglis]

Post by Craig Howland

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 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.

Post by Craig Howland
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.
"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."
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.

[Craig Howland]

Post by Brian Inglis

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.
"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.)

Post by Brian Inglis

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

[Corinna Vinschen]

...

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
[...]
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

[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

[Takashi Yano]

Hi,

On Thu, 5 Jul 2018 13:38:52 +0200

Post by Andreas Köpke
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.

[Andreas Köpke]

ah sorry -- unistd.h works.

Post by Takashi Yano
Hi,
On Thu, 5 Jul 2018 13:38:52 +0200

Post by Andreas Köpke
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.