[PATCH 01/15] stdio-common: Don't read real input beyond the field width in scanf [BZ #13988]

[Maciej W. Rozycki]

On Tue, 20 May 2025, Florian Weimer wrote:

> >  No need or desire to set `errno' here; it's not used anyway.  So it'll 
> > just be:
> >
> >   if (avail == 0)
> >     {
> >       c = EOF;
> >       break;
> >     }
> >   if (inchar () == EOF)
> >     break;
> >
> > across.  Or, hmm, maybe:
> >
> >   if (avail == 0 || inchar () == EOF)
> >     {
> >       c = EOF;
> >       break;
> >     }
> >
> > -- letting the compiler figure out that `c' will have been set by `inchar' 
> > to EOF already and optimise duplicate code accordingly.
> 
> The latter seems quite reasonable to me.

 However sadly both variants result in worse machine code produced:

$ size -G stdio-common/vfscanf-internal-?.o
      text       data        bss      total filename
     27248        303          0      27551 stdio-common/vfscanf-internal-0.o
     28036        303          0      28339 stdio-common/vfscanf-internal-1.o
     27264        303          0      27567 stdio-common/vfscanf-internal-2.o
$ 

I can see extra register spills even with the second replacement variant 
(I didn't bother to look through the first one as grossly worse).  This is 
with GCC 14 on `powerpc64le-linux-gnu'.

 Would factoring out the (avail == 0 && (c = EOF)) expression to a macro 
with a side effect (analogous to that of `inchar') be an acceptable 
compromise?

 Or shall we just choose to live with it and instead challenge the GCC 
optimiser to produce better code, given that the code quality regression 
isn't that substantial?

> >  So this `c = EOF' assignment is needed precisely for that call: so that 
> > `ungetc' does not push back the final character previously consumed that 
> > fitted in the field width and which therefore must not be available there 
> > in the input to re-retrieve for any subsequent read call from the stream.
> > Just as it doesn't push back any genuine EOF retrieved by `inchar'.
> >
> >  Have I cleared your concern here?
> 
> I wasn't aware that ungetc of EOF is required to fail without doing
> anything.  It's not documented in the manual page (but our manual
> mentions it).

 And with the local macro implementation of `ungetc' in vfscanf-internal.c 
the EOF case is literally a no-op, or alternatively `ungetc_not_eof' does 
push EOF back too for the callers that need it.

  Maciej