Re: another hash cleanup

[Jim Meyering]

Eric Blake wrote:
> According to Jim Meyering on 6/19/2009 6:02 AM:
>>> But, come to think of it, why are we even malloc'ing the new_table at all
>>> in this code path?  Maybe the better technical approach would be factoring
>>> out the table size computation from hash_initialize, and have both
>>> hash_initialize and hash_rehash first compute the needed size and later do
>>> the malloc, so that hash_rehash doesn't even malloc in the first place if
>>> the size computation shows it isn't necessary.
>>
>> Yes, I think that would be better still.
>
> Like so, along with an overflow bug fix.  OK to apply?
...
> +2009-06-19  Eric Blake  <address@hidden>
> +
> +     hash: reduce memory pressure in hash_rehash no-op case
> +     * lib/hash.c (next_prime): Avoid overflow.
> +     (hash_initialize): Factor bucket size computation...
> +     (compute_bucket_size): ...into new helper function.
> +     (hash_rehash): Use new function and open coding to reduce memory
> +     pressure, and avoids a memory leak in USE_OBSTACK code.
> +     Reported by Jim Meyering.

Looks good, but I haven't tested.
Ok, presuming you have.

One minor suggested change:

> diff --git a/lib/hash.c b/lib/hash.c
...
> @@ -847,25 +857,33 @@ hash_find_entry (Hash_table *table, const void *entry,
>  bool
>  hash_rehash (Hash_table *table, size_t candidate)
>  {
> +  Hash_table storage;
>    Hash_table *new_table;
>    struct hash_entry *bucket;
>    struct hash_entry *cursor;
>    struct hash_entry *next;
> +  size_t new_size;
>
> -  new_table = hash_initialize (candidate, table->tuning, table->hasher,
> -                            table->comparator, table->data_freer);
> -  if (new_table == NULL)
> +  new_size = compute_bucket_size (candidate, table->tuning);

Please combine the declaration and assignment:

  size_t new_size = compute_bucket_size (candidate, table->tuning);