[libvirt] [PATCH] Allow hash tables to use generic pointers as keys

[Daniel P. Berrange]

On Tue, Feb 22, 2011 at 04:16:50PM -0700, Eric Blake wrote:
> On 02/22/2011 10:54 AM, Daniel P. Berrange wrote:
> > Relax the restriction that the hash table key must be a string
> > by allowing an arbitrary hash code generator + comparison func
> > to be provided
> > 
> > * util/hash.c, util/hash.h: Allow any pointer as a key
> > * internal.h: Include stdbool.h as standard.
> > * conf/domain_conf.c, conf/domain_conf.c,
> >   conf/nwfilter_params.c, nwfilter/nwfilter_gentech_driver.c,
> >   nwfilter/nwfilter_gentech_driver.h, nwfilter/nwfilter_learnipaddr.c,
> >   qemu/qemu_command.c, qemu/qemu_driver.c,
> >   qemu/qemu_process.c, uml/uml_driver.c,
> >   xen/xm_internal.c: s/char */void */ in hash callbacks
> 
> > +++ b/src/internal.h
> > @@ -8,6 +8,7 @@
> >  # include <errno.h>
> >  # include <limits.h>
> >  # include <verify.h>
> > +# include <stdbool.h>
> 
> Should we be scrubbing other files to remove includes rendered redundant
> by including "internal.h"?

I've sent a separate patch which adds this, and scrubs other files
at the same time, and rebased this ontop of it.

> 
> > +static bool virHashStrEqual(const void *namea, const void *nameb)
> > +{
> > +    return strcmp(namea, nameb) == 0 ? true : false;
> 
> cond == 0 ? true : false always looks like overkill to me (almost like
> you were getting stumped by the 'make syntax-check' rules).  What's
> wrong with:
> 
> return STREQ(namea, nameb);

Opps, don't know why I didn't use STREQ already

> > @@ -87,8 +110,9 @@ virHashComputeKey(virHashTablePtr table, const char *name)
> >   *
> >   * Returns the newly created object, or NULL if an error occured.
> >   */
> > -virHashTablePtr
> > -virHashCreate(int size, virHashDeallocator deallocator)
> > +virHashTablePtr virHashCreateFull(int size, virHashDeallocator deallocator,
> 
> Why the style change? I would have expected a newline between return
> type and function name:
> 
> virHashTablePtr
> virHashCreateFull(int size, ...
> 
> ACK with those nits addressed.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|