On 04/19/2011 07:22 AM, Jiri Denemark wrote:
This adds several tests for remaining hash APIs (custom hasher/comparator functions are not covered yet, though).
All tests pass both before and after the "Simplify hash implementation". --- src/util/hash.c | 18 +++ src/util/hash.h | 1 + tests/hashdata.h | 237 +++++++++++++++++++++++++++++++++++- tests/hashtest.c | 361 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 615 insertions(+), 2 deletions(-)
diff --git a/src/util/hash.c b/src/util/hash.c index fc7652d..5dbc7f1 100644 --- a/src/util/hash.c +++ b/src/util/hash.c @@ -499,6 +499,24 @@ virHashSize(virHashTablePtr table) }
/** + * virHashTableSize: + * @table: the hash table + * + * Query the size of the hash @table, i.e., number of keys in the table.
This is confusing. I think virHashSize is already the number of keys (or elements) in the table; whereas virHashTableSize is the number of buckets (> keys if table is not full, < keys if table is using lots of collision chains but hasn't had a reason to grow). I'd rather use the term "buckets" than "keys" for this documentation.
+ * + * Returns the number of keys in the hash table or + * -1 in case of error + */ +int +virHashTableSize(virHashTablePtr table) +{ + if (table == NULL) + return -1; + return table->size; +}
Nice, but does it also need an addition to src/libvirt_private.syms?
"d1a564b2-c7f3-4b76-8712-3b8f5aae6ded", - "0e614f33-c1da-4cfe-b6d5-65ecd2d066f2" + "0e614f33-c1da-4cfe-b6d5-65ecd2d066f2", +/* [250] */ "334fdaba-f373-42ff-8546-219c1463a7c5", + "d4ff408e-8d43-46ff-94a4-bcfa6c994776", +/* [253] */ "d1abd887-d5de-46b0-88d6-f71f31a61305", +/* [254] */ "1d76af65-64d6-4211-b1b5-f5b799595e4d", +/* [255] */ "b3ad4257-29b0-4c44-b7a7-95f1d102769c"
Put a trailing comma on this one (if you had done that last time, we wouldn't have the - vs. + line for 0e614f33... a few lines earlier).
+const char *uuids_new[] = { + "a103cc42-d0e5-40fb-8f7f-3c1bee93e327", + "01e13dc5-e65b-4988-a0cc-0d2f1f1e10fe", + "71f3678a-a8c6-4176-a26e-c34779067135", + "4f054508-22d5-49e1-9962-7508225c8b16", + "e572116b-5b7b-45fd-bbe9-296029ce16b5", + "695c8cfe-9830-4d9a-be67-50a124cefb76", + "ea244996-645b-4a19-ad4a-48f3022b8e94", + "0fd98758-9cc4-4779-b403-95ae3500f138", + "b86772b4-0728-46ae-99e8-027799697838", + "1c0cd559-81cd-4c27-8e24-6aef6f5af7f1", + "2a37fe4a-8825-4fd6-9284-e1606967548a", + "5920cc9d-62a3-4772-9e73-eb97f0bc483c", + "53c215dd-bdba-4fdc-887a-86ab6f860df4"
Trailing comma here, too.
+static int +testHashRemoveSetIter(const void *payload ATTRIBUTE_UNUSED, + const void *name, + const void *data) +{ + int *count = (int *) data; + bool rem = false; + int i; + + for (i = 0; i < ARRAY_CARDINALITY(uuids_subset); i++) { + if (STREQ(uuids_subset[i], name)) { + rem = true; + break; + } + } + + if (rem || rand() % 2) {
Do we need to seed the random number pseudo-generator, so that the test is reproducible? I guess I'm okay if it's not seeded, though. Overall, this looks like a good patch; the fact that it passes 'make check' speaks highly, and it is some good additional coverage. ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org