On 21-07-14 17:48, Linus Torvalds wrote:
On Mon, Jul 21, 2014 at 2:08 AM, Anton Lundin glance@acc.umu.se wrote:
This fixes a always false comparison that a unsigned int can never be below zero. This was found when compiling with clang.
.. and I think clang is pure and utter shit in that case.
The thing is, just look at your patch:
@@ -142,7 +142,7 @@ dc_ihex_file_read (dc_ihex_file_t *file, dc_ihex_entry_t *entry)
/* Get the record type. */ type = data[3];
if (type < 0 || type > 5) {
if (type > 5) { ERROR (file->context, "Invalid record type(0x%02x).", type); return DC_STATUS_DATAFORMAT; }
and ask yourself: "Which version is better? Before or after the patch?"
The original code is better and clearer. It clearly does a range comparison, and it clearly verifies that "type" is between 0-5. Agreed?
I agree with Linus here. The original code is just easier to understand. Note that there are also a few other places in the code where there is a comparison like this. Strange that clang didn't find those too. Anyway, I prefer to keep this for the reasons Linus explained.
BTW, gcc also warns about this when compiled with -Wtype-limits (included in -Wextra).
Jef