[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hash: silence spurious clang warning
From: |
Jim Meyering |
Subject: |
Re: [PATCH] hash: silence spurious clang warning |
Date: |
Tue, 31 Aug 2010 08:44:41 +0200 |
Bruno Haible wrote:
> Hi Eric,
>
>> - if (! (bucket < table->bucket_limit))
>> + if (! (bucket && bucket < table->bucket_limit))
>> abort ();
>
> I would not apply this, because it causes a slowdown at runtime
> for no good reason.
>
> I think the paragraph that Paul cited just three hours ago
>
> "Don't make the program ugly to placate `lint'. Please don't insert
> any casts to `void'. Zero without a cast is perfectly fine as a null
> pointer constant, except when calling a varargs function."
>
> in a certain sense also applies to 'clang'. Don't make the program
> ugly or less efficient to placate 'clang', because it's not a tool that
> we use daily.
>
>> Clang assumed that the for loop at line 310 is skipped because
>> cursor is NULL, which implies bucket is NULL, which implies
>> that line 316 bucket->data is a dereference near NULL. But
>> this is invalid, because bucket was explicitly initialized
>> to table->bucket (non-NULL) plus some offset, at line 302.
>
> Given this analysis, I would rewrite the code as below. This should
> not only placate clang's warning, but also make the code faster
> rather than slower.
>
> 2010-08-30 Bruno Haible <address@hidden>
>
> * lib/hash.c (hash_get_next): Remove unnecessary test against NULL.
> Reported by Eric Blake.
Thanks to both of you.
I've pushed your change, Bruno.
Re: [PATCH] hash: silence spurious clang warning,
Jim Meyering <=