[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Replace hashing algorithm with murmurhash



On Wed, Jan 25, 2012 at 09:55:25AM -0700, Eric Blake wrote:
> On 01/25/2012 09:38 AM, Daniel P. Berrange wrote:
> >>> -static unsigned long virCgroupPidCode(const void *name)
> >>> +static int32_t virCgroupPidCode(const void *name, int32_t seed)
> >>>  {
> >>> -    return (unsigned long)name;
> >>> +    unsigned long pid = (unsigned long)name;
> >>> +    return virHashCodeGen(&pid, sizeof(pid), seed);
> >>
> >> I'm not sure if it matters, but you could shorten these two lines to
> >> just one:
> >>
> >> return virHashCodeGen(&name, sizeof(unsigned long), seed);
> > 
> > I actually preferred the slightly more verbose style, to make
> > it clearer that we're actually passing in the PID as an int,
> > cast to a pointer, instead of an int pointer.
> 
> And given the recent thread about mingw32, this is wrong anyways.
> There, 'unsigned long' is 32 bits, but pid_t is 64 bits, so we'd be
> losing information in our hash.  We probably need to fix this code to be
> passing (void *)&pid_t and dereferencing the pointer to get to a full
> pid, rather than cheating with (void*)(unsigned long)pid_t and possibly
> losing bits (although since this is in a context of hashing, lost bits
> only means more chance for collision, and not a fatal algorithmic flaw).

Actually given the context of this usage, we don't need to use pid_t
here. The values come from doing  scanf("%lu") on the cgroups task
file, and this will only ever run on a Linux host.

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]