fprintf() crashes on wide-oriented stream.
Commit Message
Hi,
I noticed that the following test case crashes at printf() with current
newlib.
#include <stdio.h>
#include <wchar.h>
#include <locale.h>
int main()
{
setlocale(LC_ALL, "C.UTF-8");
wprintf(L"%ls\n", L"aaaa"); /* or fwide(stdout, 1); */
printf("%ls\n", L"bbbb"); /* <--- crash here */
return 0;
}
I looked into this problem and found the cause.
A narrow char string which can be odd bytes in length is cast into
a wide char string which should be even bytes in length in __sprint_r/
__sfputs_r based on the __SWID flag. As a result, if the length is
odd bytes, the reading buffer runs over the buffer length, which causes
a crash. If the length is even bytes, crash does not happen, but garbage
is printed. This hapens if printf("%ls\r\n", L"bbbb"); is used instead.
^^
The same issue seemed to be reported ten years ago.
https://sourceware.org/pipermail/newlib/2013/010831.html
I have built a patch attached for this issue.
With this patch, __sfputs_r/__sprint_r is split into two versions, one
is for vfprintf which does not handle wide string, and the other (newly
introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
string. Please note that fprintf gets working for wide orient stream
just like BSD libc, which behaves differently from GNU libc.
This patch also fixes nano-vfprintf.c as well as vfprintf.c/vfwprintf.c
in the same manner.
Could someone please review the patch?
Thanks in advance.
Comments
On Tue, 26 Sep 2023 12:41:47 +0900
Takashi Yano wrote:
> Hi,
>
> I noticed that the following test case crashes at printf() with current
> newlib.
>
>
> #include <stdio.h>
> #include <wchar.h>
> #include <locale.h>
>
> int main()
> {
> setlocale(LC_ALL, "C.UTF-8");
> wprintf(L"%ls\n", L"aaaa"); /* or fwide(stdout, 1); */
> printf("%ls\n", L"bbbb"); /* <--- crash here */
> return 0;
> }
>
>
> I looked into this problem and found the cause.
>
> A narrow char string which can be odd bytes in length is cast into
> a wide char string which should be even bytes in length in __sprint_r/
> __sfputs_r based on the __SWID flag. As a result, if the length is
> odd bytes, the reading buffer runs over the buffer length, which causes
> a crash. If the length is even bytes, crash does not happen, but garbage
> is printed. This hapens if printf("%ls\r\n", L"bbbb"); is used instead.
> ^^
>
> The same issue seemed to be reported ten years ago.
> https://sourceware.org/pipermail/newlib/2013/010831.html
>
> I have built a patch attached for this issue.
>
> With this patch, __sfputs_r/__sprint_r is split into two versions, one
> is for vfprintf which does not handle wide string, and the other (newly
> introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
> string. Please note that fprintf gets working for wide orient stream
> just like BSD libc, which behaves differently from GNU libc.
>
> This patch also fixes nano-vfprintf.c as well as vfprintf.c/vfwprintf.c
> in the same manner.
v2: Remove __sprint_r from nano-vfprintf.c which does not seem to be used
anymore.
On Tue, 26 Sep 2023 12:41:47 +0900
Takashi Yano wrote:
> With this patch, __sfputs_r/__sprint_r is split into two versions, one
> is for vfprintf which does not handle wide string, and the other (newly
> introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
> string. Please note that fprintf gets working for wide orient stream
> just like BSD libc, which behaves differently from GNU libc.
I confirmed musl libc also behaves as BSD libc.
In the GNU libc (glibc), fprintf() returns -1 with no errno set if the
stream is wide-oriented.
On Thu, 28 Sep 2023 12:58:27 +0900
Takashi Yano wrote:
> On Tue, 26 Sep 2023 12:41:47 +0900
> Takashi Yano wrote:
> > With this patch, __sfputs_r/__sprint_r is split into two versions, one
> > is for vfprintf which does not handle wide string, and the other (newly
> > introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
> > string. Please note that fprintf gets working for wide orient stream
> > just like BSD libc, which behaves differently from GNU libc.
>
> I confirmed musl libc also behaves as BSD libc.
>
> In the GNU libc (glibc), fprintf() returns -1 with no errno set if the
> stream is wide-oriented.
I also confirmed that Solaris libc behaves as same as BSD libc.
I wonder what are the advantages of the glibc implementation?
Any historical reason?
On 2023-09-28 02:42, Takashi Yano wrote:
> On Thu, 28 Sep 2023 12:58:27 +0900
> Takashi Yano wrote:
>> On Tue, 26 Sep 2023 12:41:47 +0900
>> Takashi Yano wrote:
>>> With this patch, __sfputs_r/__sprint_r is split into two versions, one
>>> is for vfprintf which does not handle wide string, and the other (newly
>>> introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
>>> string. Please note that fprintf gets working for wide orient stream
>>> just like BSD libc, which behaves differently from GNU libc.
Note that glibc handles %c/%s in wchar_t functions and %C/%lc/%S/%ls in char
functions: see `man -m linux 3 fwide` NOTES.
>> I confirmed musl libc also behaves as BSD libc.
>>
>> In the GNU libc (glibc), fprintf() returns -1 with no errno set if the
>> stream is wide-oriented.
That is not documented in released man-pages-linux.
What is documented is that the width setting persists once set, and can not be
changed until the stream is closed.
> I also confirmed that Solaris libc behaves as same as BSD libc.
>
> I wonder what are the advantages of the glibc implementation?
> Any historical reason?
What are the behavioural differences you perceive?
There is a public inbox you can search at
https://inbox.sourceware.org/libc-help/
and you can ask mailto:libc-help@sourceware.org about anything.
Ping?
Is this Corinna's domain?
On Tue, 26 Sep 2023 17:30:13 +0900
Takashi Yano wrote:
> On Tue, 26 Sep 2023 12:41:47 +0900
> Takashi Yano wrote:
> > Hi,
> >
> > I noticed that the following test case crashes at printf() with current
> > newlib.
> >
> >
> > #include <stdio.h>
> > #include <wchar.h>
> > #include <locale.h>
> >
> > int main()
> > {
> > setlocale(LC_ALL, "C.UTF-8");
> > wprintf(L"%ls\n", L"aaaa"); /* or fwide(stdout, 1); */
> > printf("%ls\n", L"bbbb"); /* <--- crash here */
> > return 0;
> > }
> >
> >
> > I looked into this problem and found the cause.
> >
> > A narrow char string which can be odd bytes in length is cast into
> > a wide char string which should be even bytes in length in __sprint_r/
> > __sfputs_r based on the __SWID flag. As a result, if the length is
> > odd bytes, the reading buffer runs over the buffer length, which causes
> > a crash. If the length is even bytes, crash does not happen, but garbage
> > is printed. This hapens if printf("%ls\r\n", L"bbbb"); is used instead.
> > ^^
> >
> > The same issue seemed to be reported ten years ago.
> > https://sourceware.org/pipermail/newlib/2013/010831.html
> >
> > I have built a patch attached for this issue.
> >
> > With this patch, __sfputs_r/__sprint_r is split into two versions, one
> > is for vfprintf which does not handle wide string, and the other (newly
> > introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
> > string. Please note that fprintf gets working for wide orient stream
> > just like BSD libc, which behaves differently from GNU libc.
> >
> > This patch also fixes nano-vfprintf.c as well as vfprintf.c/vfwprintf.c
> > in the same manner.
>
> v2: Remove __sprint_r from nano-vfprintf.c which does not seem to be used
> anymore.
>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>
I'll look at it.
-- Jeff J.
On Tue, Oct 3, 2023 at 4:30 AM Takashi Yano <takashi.yano@nifty.ne.jp>
wrote:
> Ping?
>
> Is this Corinna's domain?
>
> On Tue, 26 Sep 2023 17:30:13 +0900
> Takashi Yano wrote:
> > On Tue, 26 Sep 2023 12:41:47 +0900
> > Takashi Yano wrote:
> > > Hi,
> > >
> > > I noticed that the following test case crashes at printf() with current
> > > newlib.
> > >
> > >
> > > #include <stdio.h>
> > > #include <wchar.h>
> > > #include <locale.h>
> > >
> > > int main()
> > > {
> > > setlocale(LC_ALL, "C.UTF-8");
> > > wprintf(L"%ls\n", L"aaaa"); /* or fwide(stdout, 1); */
> > > printf("%ls\n", L"bbbb"); /* <--- crash here */
> > > return 0;
> > > }
> > >
> > >
> > > I looked into this problem and found the cause.
> > >
> > > A narrow char string which can be odd bytes in length is cast into
> > > a wide char string which should be even bytes in length in __sprint_r/
> > > __sfputs_r based on the __SWID flag. As a result, if the length is
> > > odd bytes, the reading buffer runs over the buffer length, which causes
> > > a crash. If the length is even bytes, crash does not happen, but
> garbage
> > > is printed. This hapens if printf("%ls\r\n", L"bbbb"); is used instead.
> > > ^^
> > >
> > > The same issue seemed to be reported ten years ago.
> > > https://sourceware.org/pipermail/newlib/2013/010831.html
> > >
> > > I have built a patch attached for this issue.
> > >
> > > With this patch, __sfputs_r/__sprint_r is split into two versions, one
> > > is for vfprintf which does not handle wide string, and the other (newly
> > > introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
> > > string. Please note that fprintf gets working for wide orient stream
> > > just like BSD libc, which behaves differently from GNU libc.
> > >
> > > This patch also fixes nano-vfprintf.c as well as vfprintf.c/vfwprintf.c
> > > in the same manner.
> >
> > v2: Remove __sprint_r from nano-vfprintf.c which does not seem to be used
> > anymore.
> >
> > --
> > Takashi Yano <takashi.yano@nifty.ne.jp>
>
>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>
>
>
On 2023-10-03 02:30, Takashi Yano wrote:
> Ping? Is this Corinna's domain?
https://sourceware.org/newlib/
Patience - the volunteer reviewers may be busy with work or life or be on
vacation for a month!
On Tue, 3 Oct 2023 13:31:30 -0400
Jeff Johnston wrote:
> I'll look at it.
Thanks!
Hi Takashi,
I finally took a look at this. The issue is whether POSIX compliance is
desired. Corinna would have
strong opinions that it is desired and thus, I think she should have her
say when she gets back. I personally believe that
newlib should have behaved like glibc. I also think the test snippet is
invalid and should have performed an fwide call on stdout
to reset the wide-orientation and have the code work properly in all cases.
-- Jeff J.
On Wed, Sep 27, 2023 at 11:58 PM Takashi Yano <takashi.yano@nifty.ne.jp>
wrote:
> On Tue, 26 Sep 2023 12:41:47 +0900
> Takashi Yano wrote:
> > With this patch, __sfputs_r/__sprint_r is split into two versions, one
> > is for vfprintf which does not handle wide string, and the other (newly
> > introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
> > string. Please note that fprintf gets working for wide orient stream
> > just like BSD libc, which behaves differently from GNU libc.
>
> I confirmed musl libc also behaves as BSD libc.
>
> In the GNU libc (glibc), fprintf() returns -1 with no errno set if the
> stream is wide-oriented.
>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>
>
>
Hi Jeff,
Thanks for reviewing and the comment.
On Wed, 4 Oct 2023 16:16:13 -0400
Jeff Johnston wrote:
> I finally took a look at this. The issue is whether POSIX compliance is
> desired.
IIUC, POSIX states that width setting is once decided, it cannot be
changed until the stream is closed. However, nothing is stated what
should happen when different width data is output into the stream.
> Corinna would have
> strong opinions that it is desired and thus, I think she should have her
> say when she gets back. I personally believe that
> newlib should have behaved like glibc. I also think the test snippet is
> invalid and should have performed an fwide call on stdout
> to reset the wide-orientation and have the code work properly in all cases.
Currently, fputs and fputc works even for wide-oriended stream, so to
be consistent with that, fprintf also might be better to work.
I wouldn't necessarily expect fprintf to work on wide-oriented streams,
but buffer overruns should not happen anyway.
So, newlib should be fixed either way.
On Thu, 5 Oct 2023 19:18:14 +0900
Takashi Yano wrote:
> Hi Jeff,
>
> Thanks for reviewing and the comment.
>
> On Wed, 4 Oct 2023 16:16:13 -0400
> Jeff Johnston wrote:
> > I finally took a look at this. The issue is whether POSIX compliance is
> > desired.
>
> IIUC, POSIX states that width setting is once decided, it cannot be
> changed until the stream is closed. However, nothing is stated what
> should happen when different width data is output into the stream.
>
> > Corinna would have
> > strong opinions that it is desired and thus, I think she should have her
> > say when she gets back. I personally believe that
> > newlib should have behaved like glibc. I also think the test snippet is
> > invalid and should have performed an fwide call on stdout
> > to reset the wide-orientation and have the code work properly in all cases.
>
> Currently, fputs and fputc works even for wide-oriended stream, so to
> be consistent with that, fprintf also might be better to work.
>
> I wouldn't necessarily expect fprintf to work on wide-oriented streams,
> but buffer overruns should not happen anyway.
>
> So, newlib should be fixed either way.
As a test, I made a patch attached to make it behave like glibc.
What do you think?
On 2023-10-05 17:18, Takashi Yano wrote:
> On Thu, 5 Oct 2023 19:18:14 +0900
> Takashi Yano wrote:
>> Hi Jeff,
>>
>> Thanks for reviewing and the comment.
>>
>> On Wed, 4 Oct 2023 16:16:13 -0400
>> Jeff Johnston wrote:
>>> I finally took a look at this. The issue is whether POSIX compliance is
>>> desired.
>>
>> IIUC, POSIX states that width setting is once decided, it cannot be
>> changed until the stream is closed. However, nothing is stated what
>> should happen when different width data is output into the stream.
>>
>>> Corinna would have
>>> strong opinions that it is desired and thus, I think she should have her
>>> say when she gets back. I personally believe that
>>> newlib should have behaved like glibc. I also think the test snippet is
>>> invalid and should have performed an fwide call on stdout
>>> to reset the wide-orientation and have the code work properly in all cases.
>>
>> Currently, fputs and fputc works even for wide-oriended stream, so to
>> be consistent with that, fprintf also might be better to work.
>>
>> I wouldn't necessarily expect fprintf to work on wide-oriented streams,
>> but buffer overruns should not happen anyway.
>>
>> So, newlib should be fixed either way.
>
> As a test, I made a patch attached to make it behave like glibc.
> What do you think?
Can you please define symbols for those 1 and -1 so that the code would
be easier to understand when reading it?
Hey guys,
On Oct 4 16:16, Jeff Johnston wrote:
> Hi Takashi,
>
> I finally took a look at this. The issue is whether POSIX compliance is
> desired. Corinna would have
> strong opinions that it is desired and thus, I think she should have her
> say when she gets back. I personally believe that
> newlib should have behaved like glibc.
I took a look into the POSIX docs. POSIX aligns with ISO/IEC 9899:1999.
The description is slightly vague, in that it only has to say this:
"Byte input/output functions cannot be applied to a wide-oriented
stream, and wide-character input/output functions cannot be applied to
a byte-oriented stream."
It does not explicitely outline what "cannot be applied" means in this
context.
IIUC, this *could* mean that in case of the testcase a crash is as much
standards-compliant as the GLibC behaviour. Not that a crash is desired,
of course...
In how far the BSD behaviour is covered by this description, I really
can't tell.
I wonder if the Austin group could clarify, or if a clarification
already exists and I just missed it. CC Eric, in case he wants to
follow up on this.
Either way, I think that the safe way forward is actually to behave
as GLibC does.
> I also think the test snippet is
> invalid and should have performed an fwide call on stdout
> to reset the wide-orientation and have the code work properly in all cases.
In terms of ISO/IEC 9899:1999 I agree. However, it also shows the
flaw that newlib crashes with a buffer overflow, which we should
avoid if possible.
Does that make sense?
Thanks,
Corinna
On 2023-11-02 12:53, Corinna Vinschen wrote:
> On Oct 4 16:16, Jeff Johnston wrote:
>> I finally took a look at this. The issue is whether POSIX compliance is
>> desired. Corinna would have strong opinions that it is desired and thus,
>> I think she should have her say when she gets back. I personally believe
>> that newlib should have behaved like glibc.
> I took a look into the POSIX docs. POSIX aligns with ISO/IEC 9899:1999.
> The description is slightly vague, in that it only has to say this:
> "Byte input/output functions cannot be applied to a wide-oriented
> stream, and wide-character input/output functions cannot be applied to
> a byte-oriented stream."
> It does not explicitely outline what "cannot be applied" means in this
> context.
That seems to me to imply that the compiler /could/ diagnose if oriented stream
functions are used consistently, and that those explicitly byte- or
wide-oriented stream I/O functions should check the orientation and fail, or do
nothing, unless they are required to be flexible like the perror/psig...
functions mentioned below, to adapt and not change the stream orientation.
> IIUC, this *could* mean that in case of the testcase a crash is as much
> standards-compliant as the GLibC behaviour. Not that a crash is desired,
> of course...
> In how far the BSD behaviour is covered by this description, I really
> can't tell.
> I wonder if the Austin group could clarify, or if a clarification
> already exists and I just missed it. CC Eric, in case he wants to
> follow up on this.
> Either way, I think that the safe way forward is actually to behave
> as GLibC does.
As far as I could see, the only obvious changes are tweaks for exceptions about
open_wmemstream.
It also looks like errno should be set to EBADF to indicate that the operation
is invalid for the stream.
>> I also think the test snippet is invalid and should have performed an fwide
>> call on stdout to reset the wide-orientation and have the code work
>> properly in all cases.
That should not be necessary, as the first byte- or wide-oriented stream I/O
function called against the stream after fopen or freopen sets the orientation
from unoriented; that includes fwide() if called with a non-zero argument;
otherwise fwide() may query the stream orientation but not change it after it is
set.
> In terms of ISO/IEC 9899:1999 I agree. However, it also shows the
> flaw that newlib crashes with a buffer overflow, which we should
> avoid if possible.
Also note from Takashi's issue raised by Eric 5 years ago, perror(), psignal(),
and psiginfo() "shall not change the orientation of the standard error stream":
https://collaboration.opengroup.org/operational/mailarch.php?soph=N&action=show&archive=austin-group-l&num=27231&limit=100&offset=9400&sid=
https://www.mail-archive.com/austin-group-l@opengroup.org/msg02582.html
Hi Takashi, hi Jeff,
I checked the history of the orientation stuff and I think I came to a
conclusion.
On Oct 6 00:18, Takashi Yano wrote:
> On Thu, 5 Oct 2023 19:18:14 +0900
> Takashi Yano wrote:
> > Hi Jeff,
> >
> > Thanks for reviewing and the comment.
> >
> > On Wed, 4 Oct 2023 16:16:13 -0400
> > Jeff Johnston wrote:
> > > I finally took a look at this. The issue is whether POSIX compliance is
> > > desired.
> >
> > IIUC, POSIX states that width setting is once decided, it cannot be
> > changed until the stream is closed. However, nothing is stated what
> > should happen when different width data is output into the stream.
> >
> > > Corinna would have
> > > strong opinions that it is desired and thus, I think she should have her
> > > say when she gets back. I personally believe that
> > > newlib should have behaved like glibc. I also think the test snippet is
> > > invalid and should have performed an fwide call on stdout
> > > to reset the wide-orientation
You can't reset the orientation once it's set. fwide(3) only allows
to change the orientation if it's still undecided. Once you did
set the orientation, you can't change it back, neither to undecided,
nor to the other orientation. freopen(3) is the only way to reset
the orientation to undecided.
> and have the code work properly in all cases.
> >
> > Currently, fputs and fputc works even for wide-oriended stream, so to
> > be consistent with that, fprintf also might be better to work.
> >
> > I wouldn't necessarily expect fprintf to work on wide-oriented streams,
> > but buffer overruns should not happen anyway.
> >
> > So, newlib should be fixed either way.
>
> As a test, I made a patch attached to make it behave like glibc.
> What do you think?
It took me a while, but I think the BSD behaviour is only accepted
(acceptable) due its long history. The only really correct way of
handling this issue is to do soemthing along the lines of GLibC.
I. e., while "cannot be applied" is sufficently vague, it should be
interpreted as "must not be applied", basically. However, "must not"
kind of implies setting errno, but there's not a trace of that in
the standard.
Consequentially, IMHO, the way GLibC handles it sounds like the best way
out: The call is a no-op and returns a value indicating that the stream
isn't available for the given operation (EOF/WEOF/younameit), but it
does not change errno. There's no errno value defined for this kind
of problem anyway.
Takashi, assuming everybody is ok, your patch below looks good. I'm
just wondering if we really need an extra QUERY_ORIENT() macro. What if
ORIENT() itself returns the actual value? That would allow to just
write, for instance
- ORIENT(fp, 1);
+ if (ORIENT(fp, 1) != 1)
+ return WEOF;
Thanks,
Corinna
On 2023-11-07 06:24, Corinna Vinschen wrote:
> Hi Takashi, hi Jeff,
>
>
> I checked the history of the orientation stuff and I think I came to a
> conclusion.
>
> On Oct 6 00:18, Takashi Yano wrote:
>> On Thu, 5 Oct 2023 19:18:14 +0900
>> Takashi Yano wrote:
>>> Hi Jeff,
>>>
>>> Thanks for reviewing and the comment.
>>>
>>> On Wed, 4 Oct 2023 16:16:13 -0400
>>> Jeff Johnston wrote:
>>>> I finally took a look at this. The issue is whether POSIX compliance is
>>>> desired.
>>>
>>> IIUC, POSIX states that width setting is once decided, it cannot be
>>> changed until the stream is closed. However, nothing is stated what
>>> should happen when different width data is output into the stream.
>>>
>>>> Corinna would have
>>>> strong opinions that it is desired and thus, I think she should have her
>>>> say when she gets back. I personally believe that
>>>> newlib should have behaved like glibc. I also think the test snippet is
>>>> invalid and should have performed an fwide call on stdout
>>>> to reset the wide-orientation
>
> You can't reset the orientation once it's set. fwide(3) only allows
> to change the orientation if it's still undecided. Once you did
> set the orientation, you can't change it back, neither to undecided,
> nor to the other orientation. freopen(3) is the only way to reset
> the orientation to undecided.
>
>> and have the code work properly in all cases.
>>>
>>> Currently, fputs and fputc works even for wide-oriended stream, so to
>>> be consistent with that, fprintf also might be better to work.
>>>
>>> I wouldn't necessarily expect fprintf to work on wide-oriented streams,
>>> but buffer overruns should not happen anyway.
>>>
>>> So, newlib should be fixed either way.
>>
>> As a test, I made a patch attached to make it behave like glibc.
>> What do you think?
>
> It took me a while, but I think the BSD behaviour is only accepted
> (acceptable) due its long history. The only really correct way of
> handling this issue is to do soemthing along the lines of GLibC.
>
> I. e., while "cannot be applied" is sufficently vague, it should be
> interpreted as "must not be applied", basically. However, "must not"
> kind of implies setting errno, but there's not a trace of that in
> the standard.
>
> Consequentially, IMHO, the way GLibC handles it sounds like the best way
> out: The call is a no-op and returns a value indicating that the stream
> isn't available for the given operation (EOF/WEOF/younameit), but it
> does not change errno. There's no errno value defined for this kind
> of problem anyway.
POSIX disagrees with glibc and states that errno should be set (to EALREADY,
EBADF, EBADFD, EIO, ENOTSUP, EPERM, or add EORIENT or EWIDTH?) and callers need
to do "the errno shuffle":
https://pubs.opengroup.org/onlinepubs/9699919799/functions/fwide.html
"[CX] [Option Start] The fwide() function shall not change the setting of errno
if successful.
Since no return value is reserved to indicate an error, an application wishing
to check for error situations should set errno to 0, then call fwide(), then
check errno, and if it is non-zero, assume an error has occurred. [Option End]"
...
"ERRORS
The fwide() function may fail if:
[EBADF]
[CX] [Option Start] The stream argument is not a valid stream. [Option End]"
This lack of an error definition for oriented streams seems to be an omission,
probably resulting from each implementation doing their own thing.
POSIX specifies only EBADF - all should use EBADF - if POSIX disagree with the
interpretation of the intent they can clarify that in a Note in a TC.
[I would prefer ENOTTY - E-naughty - see Python (Monty) ;^> ]
On Nov 7 09:50, Brian Inglis wrote:
> On 2023-11-07 06:24, Corinna Vinschen wrote:
> > It took me a while, but I think the BSD behaviour is only accepted
> > (acceptable) due its long history. The only really correct way of
> > handling this issue is to do soemthing along the lines of GLibC.
> >
> > I. e., while "cannot be applied" is sufficently vague, it should be
> > interpreted as "must not be applied", basically. However, "must not"
> > kind of implies setting errno, but there's not a trace of that in
> > the standard.
> >
> > Consequentially, IMHO, the way GLibC handles it sounds like the best way
> > out: The call is a no-op and returns a value indicating that the stream
> > isn't available for the given operation (EOF/WEOF/younameit), but it
> > does not change errno. There's no errno value defined for this kind
> > of problem anyway.
>
> POSIX disagrees with glibc and states that errno should be set (to EALREADY,
> EBADF, EBADFD, EIO, ENOTSUP, EPERM, or add EORIENT or EWIDTH?) and callers
> need to do "the errno shuffle":
>
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/fwide.html
>
> "[CX] [Option Start] The fwide() function shall not change the setting of
> errno if successful.
>
> Since no return value is reserved to indicate an error, an application
> wishing to check for error situations should set errno to 0, then call
> fwide(), then check errno, and if it is non-zero, assume an error has
> occurred. [Option End]"
You're totally missing the point. The POSIX text is about an error
occuring in fwide(3), and the only possible error in fwide(3) is an
invalid file pointer. Given that fwide(3) doesn't define a return
value indicating an error, the above errno checking is required in
the application.
However, Takashi's input and code as well as my reply are talking about
all the other stdio functions depending on the orientation value. The
stdio function behaviour in terms of orientation has nothing to do with
the POSIX description of fwide(3) you're quoting.
Corinna
Hi Corinna,
On Tue, 7 Nov 2023 14:24:47 +0100
Corinna Vinschen wrote:
> Hi Takashi, hi Jeff,
>
>
> I checked the history of the orientation stuff and I think I came to a
> conclusion.
>
> On Oct 6 00:18, Takashi Yano wrote:
> > On Thu, 5 Oct 2023 19:18:14 +0900
> > Takashi Yano wrote:
> > > Hi Jeff,
> > >
> > > Thanks for reviewing and the comment.
> > >
> > > On Wed, 4 Oct 2023 16:16:13 -0400
> > > Jeff Johnston wrote:
> > > > I finally took a look at this. The issue is whether POSIX compliance is
> > > > desired.
> > >
> > > IIUC, POSIX states that width setting is once decided, it cannot be
> > > changed until the stream is closed. However, nothing is stated what
> > > should happen when different width data is output into the stream.
> > >
> > > > Corinna would have
> > > > strong opinions that it is desired and thus, I think she should have her
> > > > say when she gets back. I personally believe that
> > > > newlib should have behaved like glibc. I also think the test snippet is
> > > > invalid and should have performed an fwide call on stdout
> > > > to reset the wide-orientation
>
> You can't reset the orientation once it's set. fwide(3) only allows
> to change the orientation if it's still undecided. Once you did
> set the orientation, you can't change it back, neither to undecided,
> nor to the other orientation. freopen(3) is the only way to reset
> the orientation to undecided.
>
> > and have the code work properly in all cases.
> > >
> > > Currently, fputs and fputc works even for wide-oriended stream, so to
> > > be consistent with that, fprintf also might be better to work.
> > >
> > > I wouldn't necessarily expect fprintf to work on wide-oriented streams,
> > > but buffer overruns should not happen anyway.
> > >
> > > So, newlib should be fixed either way.
> >
> > As a test, I made a patch attached to make it behave like glibc.
> > What do you think?
>
> It took me a while, but I think the BSD behaviour is only accepted
> (acceptable) due its long history. The only really correct way of
> handling this issue is to do soemthing along the lines of GLibC.
>
> I. e., while "cannot be applied" is sufficently vague, it should be
> interpreted as "must not be applied", basically. However, "must not"
> kind of implies setting errno, but there's not a trace of that in
> the standard.
>
> Consequentially, IMHO, the way GLibC handles it sounds like the best way
> out: The call is a no-op and returns a value indicating that the stream
> isn't available for the given operation (EOF/WEOF/younameit), but it
> does not change errno. There's no errno value defined for this kind
> of problem anyway.
>
> Takashi, assuming everybody is ok, your patch below looks good. I'm
> just wondering if we really need an extra QUERY_ORIENT() macro. What if
> ORIENT() itself returns the actual value? That would allow to just
> write, for instance
>
> - ORIENT(fp, 1);
> + if (ORIENT(fp, 1) != 1)
> + return WEOF;
Thanks for reviewing the patch.
I revised the patch as you suggested.
Please see v4 patch I have just posted.
@@ -356,32 +356,7 @@ __sprint_r (struct _reent *ptr,
uio->uio_iovcnt = 0;
return 0;
}
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
- if (fp->_flags2 & __SWID)
- {
- struct __siov *iov;
- wchar_t *p;
- int i, len;
-
- iov = uio->uio_iov;
- for (; uio->uio_resid != 0;
- uio->uio_resid -= len * sizeof (wchar_t), iov++)
- {
- p = (wchar_t *) iov->iov_base;
- len = iov->iov_len / sizeof (wchar_t);
- for (i = 0; i < len; i++)
- {
- if (_fputwc_r (ptr, p[i], fp) == WEOF)
- {
- err = -1;
- goto out;
- }
- }
- }
- }
- else
-#endif
- err = __sfvwrite_r(ptr, fp, uio);
+ err = __sfvwrite_r(ptr, fp, uio);
out:
uio->uio_resid = 0;
uio->uio_iovcnt = 0;
@@ -407,27 +382,11 @@ __sfputs_r (struct _reent *ptr,
{
register int i;
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
- if (fp->_flags2 & __SWID)
- {
- wchar_t *p;
-
- p = (wchar_t *) buf;
- for (i = 0; i < (len / sizeof (wchar_t)); i++)
- {
- if (_fputwc_r (ptr, p[i], fp) == WEOF)
- return -1;
- }
- }
- else
-#endif
+ for (i = 0; i < len; i++)
{
- for (i = 0; i < len; i++)
- {
- /* Call __sfputc_r to skip _fputc_r. */
- if (__sfputc_r (ptr, (int)buf[i], fp) == EOF)
- return -1;
- }
+ /* Call __sfputc_r to skip _fputc_r. */
+ if (__sfputc_r (ptr, (int)buf[i], fp) == EOF)
+ return -1;
}
return (0);
}
@@ -187,7 +187,7 @@ static char *rcsid = "$Id$";
# endif
#endif
-/* The __sprint_r/__ssprint_r functions are shared between all versions of
+/* The __ssputs_r/__ssprint_r functions are shared between all versions of
vfprintf and vfwprintf. They must only be defined once, which we do in
the INTEGER_ONLY versions here. */
#ifdef STRING_ONLY
@@ -370,23 +370,9 @@ __sfputs_r (struct _reent *ptr,
{
register int i;
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
- if (fp->_flags2 & __SWID) {
- wchar_t *p;
-
- p = (wchar_t *) buf;
- for (i = 0; i < (len / sizeof (wchar_t)); i++) {
- if (_fputwc_r (ptr, p[i], fp) == WEOF)
- return -1;
- }
- } else {
-#else
- {
-#endif
- for (i = 0; i < len; i++) {
- if (_fputc_r (ptr, buf[i], fp) == EOF)
- return -1;
- }
+ for (i = 0; i < len; i++) {
+ if (_fputc_r (ptr, buf[i], fp) == EOF)
+ return -1;
}
return (0);
}
@@ -406,27 +392,7 @@ __sprint_r (struct _reent *ptr,
uio->uio_iovcnt = 0;
return (0);
}
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
- if (fp->_flags2 & __SWID) {
- struct __siov *iov;
- wchar_t *p;
- int i, len;
-
- iov = uio->uio_iov;
- for (; uio->uio_resid != 0;
- uio->uio_resid -= len * sizeof (wchar_t), iov++) {
- p = (wchar_t *) iov->iov_base;
- len = iov->iov_len / sizeof (wchar_t);
- for (i = 0; i < len; i++) {
- if (_fputwc_r (ptr, p[i], fp) == WEOF) {
- err = -1;
- goto out;
- }
- }
- }
- } else
-#endif
- err = __sfvwrite_r(ptr, fp, uio);
+ err = __sfvwrite_r(ptr, fp, uio);
out:
uio->uio_resid = 0;
uio->uio_iovcnt = 0;
@@ -155,18 +155,95 @@ int _VFWPRINTF_R (struct _reent *, FILE *, const wchar_t *, va_list);
# ifdef STRING_ONLY
# define __SPRINT __ssprint_r
# else
-# define __SPRINT __sprint_r
+# define __SPRINT __swprint_r
# endif
int __SPRINT (struct _reent *, FILE *, register struct __suio *);
#else
# ifdef STRING_ONLY
# define __SPRINT __ssputs_r
# else
-# define __SPRINT __sfputs_r
+# define __SPRINT __sfputws_r
# endif
int __SPRINT (struct _reent *, FILE *, const char *, size_t);
#endif
+
#ifndef STRING_ONLY
+#ifdef INTEGER_ONLY
+#ifndef _FVWRITE_IN_STREAMIO
+int
+__sfputws_r (struct _reent *ptr,
+ FILE *fp,
+ const char *buf,
+ size_t len)
+{
+ register int i;
+
+#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
+ wchar_t *p;
+
+ p = (wchar_t *) buf;
+ for (i = 0; i < (len / sizeof (wchar_t)); i++) {
+ if (_fputwc_r (ptr, p[i], fp) == WEOF)
+ return -1;
+ }
+#else
+ for (i = 0; i < len; i++) {
+ if (_fputc_r (ptr, buf[i], fp) == EOF)
+ return -1;
+ }
+#endif
+ return (0);
+}
+#endif
+/*
+ * Flush out all the vectors defined by the given uio,
+ * then reset it so that it can be reused.
+ */
+int
+__swprint_r (struct _reent *ptr,
+ FILE *fp,
+ register struct __suio *uio)
+{
+ register int err = 0;
+
+ if (uio->uio_resid == 0) {
+ uio->uio_iovcnt = 0;
+ return (0);
+ }
+#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
+ do {
+ struct __siov *iov;
+ wchar_t *p;
+ int i, len;
+
+ iov = uio->uio_iov;
+ for (; uio->uio_resid != 0;
+ uio->uio_resid -= len * sizeof (wchar_t), iov++) {
+ p = (wchar_t *) iov->iov_base;
+ len = iov->iov_len / sizeof (wchar_t);
+ for (i = 0; i < len; i++) {
+ if (_fputwc_r (ptr, p[i], fp) == WEOF) {
+ err = -1;
+ goto out;
+ }
+ }
+ }
+ } while (0);
+#else
+ err = __sfvwrite_r(ptr, fp, uio);
+#endif
+out:
+ uio->uio_resid = 0;
+ uio->uio_iovcnt = 0;
+ return (err);
+}
+#else /* !INTEGER_ONLY */
+#ifndef _FVWRITE_IN_STREAMIO
+int __sfputws_r (struct _reent *, FILE *, const char *buf, size_t);
+#endif
+int __swprint_r (struct _reent *, FILE *, register struct __suio *);
+#endif /* !INTEGER_ONLY */
+
#ifdef _UNBUF_STREAM_OPT
/*
* Helper function for `fprintf to unbuffered unix file': creates a