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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list