Kovarththanan Rajaratnam wrote: > Jim Meyering wrote: >> Kovarththanan Rajaratnam wrote: >>> clang detected the following false positive: >>> >>> sort.c:906:11: warning: The left operand of '<' is a garbage value >>> if (pid < 0) >>> ~~~ ^ >>> >>> src/sort.c: Add an assert indicating that pipe_fork is always called with >>> tries > 0. This allows clang to deduce that 'pid' will always be set/is >>> valid. >> >> Thank you for all of the patches. >> I too would like to address all of clang's warnings about >> coreutils, but it seems like clang is not quite mature enough >> for me to be comfortable adding the many assertions required >> to quiet its false positive warnings. > > Sadly, clang doesn't yet support interprocedural analysis, so it doesn't > reason about how the functions are called/used (which would have avoided > this particular false positive). > >> Is there any other way to make clang understand that no warning is >> required here? > > Sadly, no. It sounds as though you might be open to annotations (similar > to splint?) or am I getting the wrong message here?
I would rather avoid using annotations, since I don't see how that can scale to multiple tools. However, one compromise might be to use a new name, say foo_assert(expr), that would be defined to assert(expr) when "lint" is defined, and otherwise to (void)0. At first I though to use clang_assert, but obviously we want a more generic name, since it will probably be useful also with coverity. I'm sure someone can come up with a good name. Then, with definitions in system.h, alongside those for IF_LINT, I'd probably accept your patches. Though please do read the HACKING guidelines. One thing I noticed in your patches is that you omitted the space that we always put just before a function invocation's opening parenthesis. One twist with this proposal is that we'd have to adjust the syntax-check rule that determines whether assert.h is used.
