On 21/07/15 21:44, Jeffrey Walton wrote: > On Tue, Jul 21, 2015 at 4:06 PM, Matt Caswell <matt at openssl.org> wrote: >> >> >> On 21/07/15 20:54, Jeffrey Walton wrote: >>>>> ^ >>>>> d1_both.c: In function 'dtls1_retransmit_message': >>>>> d1_both.c:1261:9: warning: 'save_write_sequence' may be used >>>>> uninitialized in this function [-Wmaybe-uninitialized] >>>>> memcpy(s->s3->write_sequence, save_write_sequence, >>>>> ^ >>>> >>>> This one is entirely bogus. "save_write_sequence" is initialized on line >>>> 1241. The compiler just isn't clever enough to figure that out. >>> >>> Right. But we need to learn to work with our tools :) The other option >>> throws the baby out with the bath water by disabling warnings. Or, it >>> leaves the problem in places so thousands or millions of folks have to >>> look at the issue and clear it. >> >> Agree to a point. I always config with --strict-warnings to add dev team >> flags (as do the rest of the dev team). > > This is a good point. You are saying "trust the developers, they know > what is best." I'm fine with that because they really do know what's > best. No one knows the code better. > > ... Then C&A creeps in. For some companies, they have to acceptance > test libraries before using them. Its a matter of governance, polices > and procedures. If an organization's bar is lower than OpenSSL's, then > everything is fine. If the bar is higher, then its a pain pint. > > Folks like Rich Salz knows exactly what I am talking about and > experiences the pain points regularly. (I've worn Rich's hat and > walked in his shoes). > >> We could spend a huge amount of time tracking all of those down for >> little benefit. > > To play devil's advocate, To Whom? If 10,000 people each spend 15 > minutes looking at (and re-analyzing) one warning, then the community > collectively lost 4,000 man hours. 2 minutes for a dev to clear the > issue once versus 4,000 man hours seems like a very good return on > investment. > > And to be fair, I just cleared a similar warning in Crypto++: > https://github.com/weidai11/cryptopp/commit/d04b813e8b078e717992b86b8b6103db0bd2cec3. > I new damn well all those variables were initialized, and the problem > was with analyzer's inter-procedural analysis. > > For the d04b813e8 commit, I had to analyze it to ensure it was not a > legitimate squawk. But my choices after analyzing it were: (1) spend > 30 seconds on the clear-commit-push cycle; or (2) allow the community > to spend countless hours reanalyzing it, and spend countless hours > explaining the reason for the dirty compile on the mailing list > (q.v.!). I opted for (1) because it was easier on me, and > organizations don't have to worry about C&A and governance issues. > > Like I said, its learning to play well with your tools :) Well I think what your saying is that we should play well with other people's tools! My tools (and presumably the rest of the dev team's as well) don't report this warning. Collectively the dev team expect a build to work with --strict-warnings which means that there should be no warnings reported at all for the team. That's quite a high bar to reach because we've all got slightly different set ups, so every now and then we encounter a problem that one person on the team gets but others don't. No matter how high we set the bar though there is always going to be some system somewhere that still reports a warning. We are never going to be able to eradicate that. I get your point about the time people spend on looking at a warning adding up (although somehow I just don't buy the idea that 10,000 people are going to spend 15 minutes each looking at it). I'm not arguing that we shouldn't fix warnings when we become aware of them - particularly if we think there is a real problem or the warning is coming from a common tool chain. I'm just saying that pro-actively tracking them down so that it always cleanly compiles with no warnings for everyone is not likely to be a useful use of time (and is actually not realistic anyway). Matt
