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