Re: [libvirt] [PATCH V5 01/10] Add function to get hash tables key/value pairs
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(a)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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)