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

Maciej W. Rozycki macro@redhat.com
Mon May 19 20:21:11 GMT 2025


On Mon, 19 May 2025, Florian Weimer wrote:

> >> > @@ -1657,7 +1657,8 @@ __vfscanf_internal (FILE *s, const char
> >> >  			    break;
> >> >  			  else
> >> >  			    {
> >> > -			      if (avail == 0 || inchar () == EOF)
> >> > +			      if ((avail == 0 && (c = EOF))
> >> 
> >> Please avoid assignment in a conditional.
> >
> >  It is valid C and weighing pros and cons I concluded it'd be the most 
> > straightforward approach here.  Rewriting this piece such as to avoid the 
> > inline assignment will make code more complex I'm afraid.
> >
> >  I might be wrong, so may I ask for a second opinion?
> 
> Please follow Andreas' suggestion.  I think this is equivalent to

 OK.  This will expand code and given that `inchar' sets `c' implicitly, 
so it's an assignment in the conditional as well, I find it a questionable 
coding style change, but I can't be bothered enough to fight a battle over 
it here.

>   if (avail == 0)
>     {
>       c = EOF;
>       errno = inchar_errno;
>       break;
>     }

 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.

 I'll do some fiddling and repost the series with an update according to 
findings, and in the meantime will happily accept feedback to the other 
changes from this patch set.

> but I'm not sure if this makes sense.  In particular, we seem to call
> ungetc (c, s) further down?

 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?

 Thank you for your input.

  Maciej



More information about the Libc-alpha mailing list