public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
From: Takashi Yano <takashi.yano@nifty.ne.jp>
To: newlib@sourceware.org
Subject: Re: fprintf() crashes on wide-oriented stream.
Date: Wed, 8 Nov 2023 21:05:16 +0900	[thread overview]
Message-ID: <20231108210516.6493ec17b652b3149ebf770a@nifty.ne.jp> (raw)
In-Reply-To: <ZUo6nxPGMEPBZWcz@calimero.vinschen.de>

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.

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

  parent reply	other threads:[~2023-11-08 12:05 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-26  3:41 Takashi Yano
2023-09-26  8:30 ` Takashi Yano
2023-10-03  8:30   ` Takashi Yano
2023-10-03 17:31     ` Jeff Johnston
2023-10-03 20:12       ` Takashi Yano
2023-10-03 18:07     ` Brian Inglis
2023-09-28  3:58 ` Takashi Yano
2023-09-28  8:42   ` Takashi Yano
2023-09-28 20:06     ` Brian Inglis
2023-10-04 20:16   ` Jeff Johnston
2023-10-05 10:18     ` Takashi Yano
2023-10-05 15:18       ` Takashi Yano
2023-10-05 18:20         ` Torbjorn SVENSSON
2023-11-07 13:24         ` Corinna Vinschen
2023-11-07 16:50           ` Brian Inglis
2023-11-07 19:12             ` Corinna Vinschen
2023-11-08 12:05           ` Takashi Yano [this message]
2023-11-02 18:53     ` Corinna Vinschen
2023-11-06 18:26       ` Brian Inglis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231108210516.6493ec17b652b3149ebf770a@nifty.ne.jp \
    --to=takashi.yano@nifty.ne.jp \
    --cc=newlib@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).