On Tue, Jul 08, 2014 at 11:25:20AM +0200, Paul Bolle wrote: > Is the fact that this generates false positives by itself enough to > downgrade this check to W=1? > > I do not have any hard numbers to back up my claims, but I'd like to > point out that it is possible that we never see many of the warnings > that GCC correctly issues because they might only occur during local > development. Ie, the developer involved cleans up the patch before > sending out. You're assuming that a developer doesn't use W=<num> to test her/his patches. > My experience with the warnings that actually do make it into mainline Which warnings? All warnings or only the maybe-uninitialized ones? > is that quite a few are correct while the false positives tend to be > generated by a pieces of code that are complicated (I think I've seen > the warning labeled as a "code smell"). > > For example, in my local builds this warning popped up three times in > the current development cycle: > 0) fs/direct-io.c:1011:12: warning: ‘from’ may be used uninitialized in this function [-Wmaybe-uninitialized] > fs/direct-io.c:1011:12: warning: ‘to’ may be used uninitialized in this function [-Wmaybe-uninitialized] > 1) drivers/platform/x86/eeepc-laptop.c:279:10: warning: ‘value’ may be used uninitialized in this function [-Wmaybe-uninitialized] > 2) drivers/usb/misc/usb3503.c:195:11: warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized] > > Warning 0) occurs in a 150 line function, that grows over 200 lines when > the inline functions it calls are included. And I do think it's not easy > to see hwat that code does. Have you actually tried to understand what the code does and also to see that gcc simply cannot see that the to and from will never be used in the error case? It is not that hard actually. > Anyhow, see > https://lkml.org/lkml/2014/7/1/496 for my attempt to silence this > warning by simplifying this function. Terrible idea - misdesign the code just because gcc doesn't say that two variables *might* not be initialized. > See https://lkml.org/lkml/2014/7/1/150 for my patch that silences 1) by, > again, simplifying the code. Again, this is wrong on *every* level! This is perfectly sane code where value is used *only* in the success case. > And warning 2) is correct. See https://lkml.org/lkml/2014/6/2/511 for a > possible solution. That warning should actually be not "maybe" but really -Wuninitialized. > So, in summary, I believe that the signal/noise ratio actually isn't > too bad. Moreover I think that the noise isn't merely noise, as it > points to possibly problematic sections of code. It seems you're missing the point so let me explain to you what I mean: Here's the relevant snippet from the gcc manpage: -Wmaybe-uninitialized For an automatic variable, if there exists a path from the function entry to a use of the variable that is initialized, but there exist some other paths for which the variable is not initialized, the compiler emits a warning if it cannot prove the uninitialized paths are not executed at run time. These warnings are made optional because GCC is not smart enough to see all the reasons why the code might be correct in spite of appearing to have an error. Now read it again: "gcc cannot prove the uninitialized paths are not executed at run time. These warnings are made optional because GCC is not smart enough ..." And this is exactly what I'm proposing: keep the warning but downgrade it so that it doesn't generate false positives noise. I'm not arguing it is a bad check - I'm just saying it is more of a heuristic and should belong to the rest of the W= checks. Oh, moving it to W=1 has another reason - to hide it from overeager people who really want to *fix* the code and thus obfuscate it for no sensible reason what*so*f*ckin*ever! Just so that they can please the compiler which says itself it cannot be that smart! You have given perfect examples in 0) and 1) above for exactly that. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
