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

Re: [libvirt] [PATCH] util: Simplify hash implementation



On Tue, Apr 12, 2011 at 06:54:33PM +0200, Jiri Denemark wrote:
> So far first entries for each hash key are stored directly in the hash
> table while other entries mapped to the same key are linked through
> pointers. As a result of that, the code is cluttered with special
> handling for the first items.
> 
> Commit 9677cd33eea4c65d78ba463b46b8b45ed2da1709 made it possible to
> remove current entry when iterating through all hash entries. However,
> it didn't add the special first item handling which could cause libvirtd
> crash or hang.

This highlights what should be an obvious problem.... we have near zero
test coverage of the hash table class. The code is totally standalone
and it should be easy to write a test case to exercise all its methods,
and many of the edge cases. IMHO we should fix the lack of a test suite
before refactoring it again, so that we know we're actually get it right
this time :-)

Both the existing code & your new patch are complex enough, that I'm
not confident in code review alone identifying all the bugs.

Regards,
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]