fprintf() crashes on wide-oriented stream.

[Takashi Yano]

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>