On Fri, May 27, 2016 at 5:33 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > > gcc can not always figure out which code is only used in an error > condition an assignment to indirect argument is only done after the > use of IS_ERR() catches errors. In gfs2, this results in a warning > about correct code: I figured out why gcc warns. The code is correct, but gcc isn't able to tell that, because when we return "int" for the error case from get_leaf_nr(), gcc thinks that that *could* be a zero. This code: if (IS_ERR(hash)) return PTR_ERR(hash); is obviously "return non-zero" to a kernel developer (because that's how our error codes work), but to a compiler that "return PTR_ERR()" ends up casting a "long" to an "int" return value. And the compiler thinks that while the "long" was clearly non-zero and negative, the "int" it cast things to might be zero. Yes, a very smart compiler might have figured out that the IS_ERR_VALUE check also means that the value will be non-zero in "int" as well, but that actually takes value range analysis, not just "we already ttested it". And yes, the error goes away if I turn the "int" into "long" into the affected codepaths. I'm not entirely happy about your patch, because I think it makes the code worse. You fix it by effectively making gcc test the resulting value after the type conversion. I'd really prefer to figure out some way to let gcc actually understand this error handling model. Because it's not just the warning, it also generates unnecessary double tests (first comparing a 64-bit value against the error range, and then comparing the truncated 32-bit error code against zero). Making errors be "long" does fix not just the warning but also the code generation. But we've traditionally used "int" for error returns, even though the system call path then again turns it into "long" (for somewhat related reasons, actually - we want to make sure that the "int" error code is valid in all 64 bits). So we *could* just start encouraging people to use "long" for error handling. It would fix warnings and improve the results. But let me think about this a bit more. Linus -- 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