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