On Wednesday, 30 November 2016 10:59:31 CET Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virstring.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virstring.h | 3 +++
tests/virstringtest.c | 38 ++++++++++++++++++++++++++++++++++++
4 files changed, 92 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bd46e5f..b1f42f2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2465,6 +2465,7 @@ virStringListGetFirstWithPrefix;
virStringListHasString;
virStringListJoin;
virStringListLength;
+virStringListRemove;
virStringReplace;
virStringSearch;
virStringSortCompare;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index d2fb543..7d2c4c7 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -206,6 +206,56 @@ virStringListAdd(const char **strings,
/**
+ * virStringListRemove:
+ * @strings: a NULL-terminated array of strings
+ * @newStrings: new NULL-terminated array of strings
+ * @item: string to remove
+ *
+ * Creates new strings list with all strings duplicated except
+ * for every occurrence of @item. Callers is responsible for
+ * freeing both @strings and returned list.
+ *
+ * Returns the number of items in the new list (excluding NULL
+ * anchor), -1 on error.
Wouldn't it be better to return the number of items removed? After all,
if the size of the new list is needed, virStringListLength can be used.
+int
+virStringListRemove(const char **strings,
+ char ***newStrings,
+ const char *item)
+{
+ char **ret = NULL;
+ size_t i, j = 0;
+
+ for (i = 0; strings && strings[i]; i++) {
+ if (STRNEQ(strings[i], item))
+ j++;
+ }
+
+ if (!j) {
+ *newStrings = NULL;
+ return 0;
+ }
Shouldn't this produce an empty list instead? I.e. I'd expect that:
char **elems = { "foo", NULL };
char **newStrings;
int count = virStringListRemove(elems, &newStrings, "foo");
assert(count == 0);
assert(newStrings != NULL);
assert(newStrings[0] == NULL);
Ditto when trying to remove anything from an empty list.
The exception would be when strings == NULL, so virStringListRemove
should just directly set *newStrings to NULL and return 0.
+static int testRemove(const void *args)
+{
+ const struct testSplitData *data = args;
+ char **list = NULL;
+ size_t ntokens;
+ size_t i;
+ int ret = -1;
+
+ if (!(list = virStringSplitCount(data->string, data->delim,
+ data->max_tokens, &ntokens))) {
+ VIR_DEBUG("Got no tokens at all");
+ return -1;
+ }
+
+ for (i = 0; data->tokens[i]; i++) {
+ char **tmp;
+ if (virStringListRemove((const char **) list,
+ &tmp, data->tokens[i]) < 0)
IMHO the test should also check the return value is what is expected.
Thanks,
--
Pino Toscano