[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