On 11/17/2011 01:11 PM, Stefan Berger wrote:
Add a function to the virHashTable for getting an array of the hash table's key-value pairs and have the keys (optionally) sorted.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
v5: - follwed Eric Blake's suggestions: - added better description to new function - return array of key-value pairs rather than only keys
Yep, looks like it will be more reusable now. And the more I think about it, the more I've convinced myself this is a useful interface (allows you to sort the hashtable in multiple ways, based on what comparator you pass in; using a data structure to store sorted elements with hashtable lookup speeds still only gives you a single sort order).
--- src/libvirt_private.syms | 2 ++ src/util/hash.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/util/hash.h | 21 +++++++++++++++++++++
I'd still feel a bit better if we had coverage for the new function in tests/hashtest.c, but I'll let that slide to another patch; this patch already blocks enough other useful stuff that it will get some good field testing from the tck runs you do, while we work on enhancing hashtest.c.
} + +struct getKeysIter +{ + virHashKeyValuePair *sortArray; + unsigned arrayIdx; +};
Lots simpler, huh? :)
+/* + * Get the hash table's key/value pairs and have them optionally sorted. + * The returned array contains virHashSize() elements. Aditionally,
s/Aditionally/Additionally/
+ * an empty element has been added to the end of the array whose key == NULL + * also indicates the end of the array.
We don't need both "Additionally" and "also" in the same sentence. How about: s/also indicates/to indicate/
+ * The key/value pairs are only valid as long as the underlying hash + * table is not modified, i.e., keys removed or the hash table deleted.
s/keys removed or the hash table deleted/no keys are removed or inserted, and the hash table is not deleted/
+ * The caller must only free the returned array using VIR_FREE(). + * The caller must make copies of all returned keys and values if they are + * to be used somewhere else. + */ +typedef struct _virHashKeyValuePair virHashKeyValuePair; +typedef virHashKeyValuePair *virHashKeyValuePairPtr; +struct _virHashKeyValuePair { + const void *key; + const void *value; +}; +typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr , + const virHashKeyValuePairPtr );
The spacing looks awkward on these two lines; s/ \([,)]\)/\1/ ACK. No need to see a v6 on this one, as the fixes are trivial. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org