[libvirt] [PATCH v4 1/6] util: Add helper function to clean extra spaces

[John Ferlan]

On 06/28/2018 09:22 AM, Jai Singh Rana wrote:
> This patch adds string manupulation helper function which takes

manipulation

> string as input and returns string with all but one space removed
> between letters, numbers or words.
> 
> Signed-off-by: Jai Singh Rana <JaiSingh.Rana at cavium.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virstring.c     | 36 ++++++++++++++++++++++++++++++++++++
>  src/util/virstring.h     |  3 +++
>  3 files changed, 40 insertions(+)
> 

Sorry for the delay on reviewing this series. I was hoping that someone
more familiar with details of network/vf stats would jump in...

> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 5499a368c0..272e7426dd 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2891,6 +2891,7 @@ virSkipSpacesBackwards;
>  virStrcpy;
>  virStrdup;
>  virStringBufferIsPrintable;
> +virStringCleanExtraSpaces;
>  virStringEncodeBase64;
>  virStringFilterChars;
>  virStringHasChars;
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 15f367af7c..1f45b2b553 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -139,6 +139,42 @@ virStringSplit(const char *string,
>  
>  
>  /**
> + * virCleanExtraSpacesInString:

This ^^^ doesn't match...

> + * @src: original null terminated string
> + *
> + * Returns string with all spaces but one removed between words in @src
> + * string. Caller is responsible for freeing the returned string.
> + * Returns NULL if new string could not be allocated.
> + *
> + */
> +char *
> +virStringCleanExtraSpaces(char *src)

... this ^^^ name.

Consider how virStringStripControlChars handles stripping out control
characters and then modify to skip "if (c_isspace())" perhaps using
virSkipSpaces..

Since you don't really care if the line is edited "in place", consider
just moving characters, then you don't have to VIR_ALLOC_N something
that you're not trimming at the end anyway.

You should add a test to tests/virstringtest.c similar to how
other existing character stripping tests do it, but of course proving
that you're stripping spaces as you expect.

Don't forget to alter the commit message to note that you're editing the
passed string "in place" and change the function to void.

John

> +{
> +    char *dst;
> +    size_t dstlen;
> +    int src_at = 0;
> +    int dst_at;
> +
> +    dstlen = strlen(src);
> +    if (VIR_ALLOC_N(dst, dstlen) < 0)
> +        return NULL;
> +
> +    while (src[src_at] == ' ')
> +        src_at++;
> +
> +    for (dst_at = 0; src[src_at] != '\0'; src_at++) {
> +        if (src[src_at + 1] == ' ' && src[src_at] == ' ')
> +            continue;
> +        dst[dst_at] = src[src_at];
> +        dst_at++;
> +    }
> +    dst[dst_at] = '\0';
> +
> +    return dst;
> +}
> +
> +
> +/**
>   * virStringListJoin:
>   * @strings: a NULL-terminated array of strings to join
>   * @delim: a string to insert between each of the strings
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index 607ae66e99..0778bc45c8 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -37,6 +37,9 @@ char **virStringSplit(const char *string,
>                        size_t max_tokens)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>  
> +char *virStringCleanExtraSpaces(char *src)
> +    ATTRIBUTE_NONNULL(1);
> +
>  char *virStringListJoin(const char **strings,
>                          const char *delim)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>