[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