[PATCH] Unsigned int type can't be below zero

Jef Driesen jef at libdivecomputer.org
Tue Aug 5 02:55:17 PDT 2014


On 21-07-14 17:48, Linus Torvalds wrote:
> On Mon, Jul 21, 2014 at 2:08 AM, Anton Lundin <glance at 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


More information about the devel mailing list