Re: [PATCH] hash: declare some functions with the warn_unused_result attribute

[Jim Meyering]

Eric Blake wrote:
> Eric Blake <ebb9 <at> byu.net> writes:
>> Subject: [PATCH] hash: improve memory management
>>
>> * lib/hash.c (hash_delete): Free entries if resize failed.
>> (hash_insert): Don't leave entry on table when returning failure.
>> (hash_rehash): Avoid memory leak, by guaranteeing that we won't
>> allocate once we start moving entries.
>>
>> OK to apply?

Thanks!
I hope to review it tomorrow.

> I'd also like to squash this on, to fix grammar and provide some
> clarification.  In particular, m4 uses the idiom of deleting elements during
> traversal, which is provably safe under certain conditions.

...
> diff --git a/lib/hash.c b/lib/hash.c
> index a8b8123..034f80f 100644
> --- a/lib/hash.c
> +++ b/lib/hash.c
> @@ -21,7 +21,8 @@
>  /* A generic hash table package.  */
>
>  /* Define USE_OBSTACK to 1 if you want the allocator to use obstacks instead
> -   of malloc.  If you change USE_OBSTACK, you have to recompile!  */
> +   of malloc.  If you change USE_OBSTACK, you have to recompile!  Also,
> +   memory allocation failures in the obstack are not reliably tracked.  */

Long ago (like 12-15 years), in an application that made very heavy use
of hash tables, the obstack code provided a large performance benefit.
So I'm reluctant to remove it altogether.  However, it's been so long
since I last tested it that it may well have suffered from bit rot.
And with modern heap management code, it may not be useful anymore.

I've begun writing a test module, and may even find the time to test
the USE_OBSTACK code, too.  As you say, it also needs to be improved
to handle allocation failure, probably the same way remove.c now does:

  http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=a5111af33ea6a5d27