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
