Re: [PATCH 4/8] virstring: Introduce virStringListRemove

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 30, 2016 at 10:59:31AM +0100, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  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.
> + */
> +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) {

In on of the reviews I received I was advised not to use this^^ with anything
else than pointers, so I'd like to pass this advice further on.

> +        *newStrings = NULL;
> +        return 0;
> +    }
> +
> +    if (VIR_ALLOC_N(ret, j + 1) < 0)
> +        goto error;
> +
> +    for (i = 0, j = 0; strings[i]; i++) {
> +        if (STREQ(strings[i], item))
> +            continue;
> +        if (VIR_STRDUP(ret[j], strings[i]) < 0)
> +            goto error;
> +        j++;
> +    }
> +
> +    *newStrings = ret;
> +    return j;
> + error:
> +    virStringListFree(ret);
> +    return -1;
> +}
> +
> +
> +/**
>   * virStringListFree:
>   * @str_array: a NULL-terminated array of strings to free
>   *
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index da9d35c..88eacd4 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -43,6 +43,9 @@ char *virStringListJoin(const char **strings,
>  
>  char **virStringListAdd(const char **strings,
>                          const char *item);
> +int virStringListRemove(const char **strings,
> +                        char ***newStrings,
> +                        const char *item);
>  
>  void virStringListFree(char **strings);
>  void virStringListFreeCount(char **strings,
> diff --git a/tests/virstringtest.c b/tests/virstringtest.c
> index 43657c8..a421f7b 100644
> --- a/tests/virstringtest.c
> +++ b/tests/virstringtest.c
> @@ -162,6 +162,42 @@ static int testAdd(const void *args)
>  }
>  
>  
> +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)

As Pino noted, this is a functional primitive other APIs will rely on, so I
think we'd better be sure it always returns the correct size, at least I
usually don't run valgring on our test suite.

Otherwise looks perfectly fine, ACK.

Erik

> +            goto cleanup;
> +        virStringListFree(list);
> +        list = tmp;
> +        tmp = NULL;
> +    }
> +
> +    if (list && list[0]) {
> +        virFilePrintf(stderr, "Not removed all tokens: %s", list[0]);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    virStringListFree(list);
> +    return ret;
> +}
> +
> +
>  static bool fail;
>  
>  static const char *
> @@ -636,6 +672,8 @@ mymain(void)
>              ret = -1;                                                   \
>          if (virTestRun("Add " #str, testAdd, &joinData) < 0)            \
>              ret = -1;                                                   \
> +        if (virTestRun("Remove " #str, testRemove, &splitData) < 0)     \
> +            ret = -1;                                                   \
>      } while (0)
>  
>      const char *tokens1[] = { NULL };
> -- 
> 2.8.4
> 
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux