Re: [PATCH] hash: silence spurious clang warning

[Jim Meyering]

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.