Re: [PATCH] hash: silence spurious clang warning

[Bruno Haible]

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.

*** lib/hash.c.orig     Tue Aug 31 01:06:33 2010
--- lib/hash.c  Tue Aug 31 01:06:25 2010
***************
*** 307,315 ****
      abort ();
  
    /* Find next entry in the same bucket.  */
!   for (cursor = bucket; cursor; cursor = cursor->next)
!     if (cursor->data == entry && cursor->next)
!       return cursor->next->data;
  
    /* Find first entry in any subsequent bucket.  */
    while (++bucket < table->bucket_limit)
--- 307,320 ----
      abort ();
  
    /* Find next entry in the same bucket.  */
!   cursor = bucket;
!   do
!     {
!       if (cursor->data == entry && cursor->next)
!         return cursor->next->data;
!       cursor = cursor->next;
!     }
!   while (cursor != NULL);
  
    /* Find first entry in any subsequent bucket.  */
    while (++bucket < table->bucket_limit)