3 ms·

In my experience, Coverity catches a couple of terrible bugs, and about ten thousand stylistic things like, "if (dwResult >= 0 && dwResult <= WHATEVER)" (i.e. i

In my experience, Coverity catches a couple of terrible bugs, and about ten thousand stylistic things like, "if (dwResult >= 0 && dwResult <= WHATEVER)" (i.e. it complains that a DWORD value will always be >= 0, but I don't care, because I'm explicitly expressing a range to whoever maintains my code).

groby_b 15y ago
if(IN_RANGE_CLOSED(dw_result,0,WHATEVER)) Build the appropriate model for IN_RANGE, and done. Has the benefit that you explicitly state the type of range (open|closed|half-open[LR]), so you'll be a bit more likely to think about the edge cases. Is it painful? Yes. I'd rather take that pain than debugging crash reports, though. (YMMV - it certainly depends on what you are building, how large your audience is, and what the consequences of a crash are)
- georgieporgie 15y ago
  To be honest, that's really ugly to me, and it requires me to remind myself of exactly what that macro is. I'm definitely not willing to change my code style solely to satisfy a static checker's zero-impact 'bug'.
rapala 15y ago
The use of DWORD as a type should, in my opinion, already state that the value is always non-negative. Explicitly checking for it might even be a bad thing, as it can distract the reader. He might not check the type and let negative numbers pass in as arguments because he trusts that the >= 0 check handles them. (This of course might, or might not get caught by the <= WHATEVER check).
- georgieporgie 15y ago
  You seem to be arguing that, when viewing a conditional, the reader should have to verify the type of the variable in order to glean understanding of the logic. I don't like that. I don't want people to have to double-check variable types, and I want everyone from the project lead to the new college hire (who doesn't have an intrinsic understanding of DWORD implications) to, at a glance, understand that I'm checking for a range of values. Of course, all of this would be handled by a native range expression, but we don't get that in C/C++. And, as I said in an earlier comment, project- or company-specific macros just add another layer of required comprehension.

groby_b 15y ago

if(IN_RANGE_CLOSED(dw_result,0,WHATEVER)) Build the appropriate model for IN_RANGE, and done. Has the benefit that you explicitly state the type of range (open|closed|half-open[LR]), so you'll be a bit more likely to think about the edge cases. Is it painful? Yes. I'd rather take that pain than debugging crash reports, though. (YMMV - it certainly depends on what you are building, how large your audience is, and what the consequences of a crash are)

georgieporgie 15y ago
To be honest, that's really ugly to me, and it requires me to remind myself of exactly what that macro is. I'm definitely not willing to change my code style solely to satisfy a static checker's zero-impact 'bug'.

rapala 15y ago

The use of DWORD as a type should, in my opinion, already state that the value is always non-negative. Explicitly checking for it might even be a bad thing, as it can distract the reader. He might not check the type and let negative numbers pass in as arguments because he trusts that the >= 0 check handles them. (This of course might, or might not get caught by the <= WHATEVER check).

georgieporgie 15y ago
You seem to be arguing that, when viewing a conditional, the reader should have to verify the type of the variable in order to glean understanding of the logic. I don't like that. I don't want people to have to double-check variable types, and I want everyone from the project lead to the new college hire (who doesn't have an intrinsic understanding of DWORD implications) to, at a glance, understand that I'm checking for a range of values. Of course, all of this would be handled by a native range expression, but we don't get that in C/C++. And, as I said in an earlier comment, project- or company-specific macros just add another layer of required comprehension.