[libvirt] [PATCH 4/8] virstring: Introduce virStringListRemove
Pino Toscano
ptoscano at redhat.com
Wed Nov 30 10:25:55 UTC 2016
On Wednesday, 30 November 2016 10:59:31 CET Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161130/c36f7562/attachment-0001.sig>
More information about the libvir-list
mailing list