On Tue, Jun 29, 2021 at 2:47 PM Eric Blake <[email protected]> wrote: > 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.
I do not understand what you mean by "chasing the windmill", and the top result on Google Search (https://iit.adelaide.edu.au/news/list/2021/04/29/chasing-the-windmill-what-is-wrong-with-the-us-approach-on-developing-country) seems to be about developing countries. Is this some cultural reference that non-English-speaking people are not supposed to understand? Could you please use expressions for which the meaning can be easily searched on the Internet? On the topic, I agree that "making configure snippets warning-free" is not a great objective, and is quite difficult because static code analyzer software tends to report many false positive issues. But here it is a real fix and I find it inappropriate that you reply with a message which translates to "you are doing something idiot, please stop this stupid thing, static analysis is worthless". In my opinion, I can carry patches on my own which help reduce the number of false positive issues reported by static analyzer tools (without involving any upstream discussion), but when I find a bug I prefer to report it and help upstream developers to fix it rather than keeping it for myself. This is why I submitted a patch and I believe such an approach is what makes free software possible, instead of calling the work of someone else "not worth it". > 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. Sorry about this. I posted the patch from my professional email and missed the fact that my employer added a few weeks ago a configuration to GMail which automatically adds this blurb, even to full-text emails sent to mailing lists. Best regards, Nicolas
