Re: [PATCH] lib/autoconf/c.m4: fix NULL pointer dereference in _AC_LANG_IO_PROGRAM

[Eric Blake]

On Tue, Jun 29, 2021 at 09:56:44AM +0200, Nicolas IOOSS wrote:
> > >  [FILE *f = fopen ("conftest.out", "w");
> > > - return ferror (f) || fclose (f) != 0;
> > > + return !f || ferror (f) || fclose (f) != 0;
> > >  ])])
> >
> > Thank you for the bug report and the patch. The change looks correct
> > and is simple enough not to require a copyright assignment. I will
> > apply it to Autoconf trunk in the next few days.
> >
> > Our test suite does not catch this bug, which probably means it never
> > tests AC_LANG_IO_PROGRAM inside AC_RUN_IFELSE. Would you be willing to
> > write a test case, perhaps based on the configure.ac that caused you
> > to discover the bug? If you don't have time to write a test yourself,
> > could you at least tell us about how you found the bug?
> 
> Hello,
> Thanks for your quick reply! Actually I found this bug running clang's
> static code analyzer on a project named secp256k1. When using
> "scan-build -enable-checker alpha.unix.Stream", this tool spot an
> issue while running ./configure and I found out it was a real bug. For
> more details, I described on
> https://github.com/bitcoin-core/secp256k1/pull/959#issuecomment-869733706
> the exact command line I used. So the autoconf.ac file that I used is
> https://github.com/bitcoin-core/secp256k1/blob/75ce488c2a6d47bfd6ed333e3e919a98ea86139a/configure.ac

You do realize, of course, that configure code snippets often take
shortcuts or otherwise perform the bare minimum necessary to come up
with an answer that works, rather than worrying about satisfying the
demands of every compiler warning and static code analyzer out there.
Yes, making sure the fopen() succeeded is good coding practice, but in
the scope of this particular configure test, you've got MUCH bigger
problems if a 3-line program can't even perform fopen() without
succeeding.

While I'm not opposed to your patch, I don't think that chasing the
windmill of making configure snippets warning-free is going to be
worth it, because in the end, it won't change the results of those
configure tests.

But lest I sound too negative, thank you for reporting what you found,
and for providing a patch!

> The information contained in this email and in any 
> document enclosed is strictly confidential and is intended solely for the

Legalese blurbs like these are unenforceable on a publically-archived
mailing list; you may want to consider using an alternative account
that does not append your employer's text on your mails to open source
lists.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org