On Wednesday, 30 November 2016 10:59:31 CET Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@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