[libvirt] [PATCHv2 2/5] Add functions dealing with control characters in strings

[Peter Krempa]

On Tue, Apr 14, 2015 at 13:28:47 +0200, Ján Tomko wrote:
> Add virStringHasControlChars that checks if the string has
> any control characters other than \t\r\n,
> and virStringStripControlChars that removes them in-place.
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/virstring.c     | 39 +++++++++++++++++++++++++++++++++++++++
>  src/util/virstring.h     |  2 ++
>  tests/virstringtest.c    | 39 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 82 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 7166283..d37a6b5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2134,6 +2134,7 @@ virStrdup;
>  virStringArrayHasString;
>  virStringFreeList;
>  virStringFreeListCount;
> +virStringHasControlChars;
>  virStringIsEmpty;
>  virStringJoin;
>  virStringListLength;
> @@ -2143,6 +2144,7 @@ virStringSortCompare;
>  virStringSortRevCompare;
>  virStringSplit;
>  virStringSplitCount;
> +virStringStripControlChars;
>  virStringStripIPv6Brackets;
>  virStrncpy;
>  virStrndup;
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 3dad9dd..9eb48b5 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -968,3 +968,42 @@ virStringStripIPv6Brackets(char *str)
>          str[len - 2] = '\0';
>      }
>  }
> +
> +static const char control_chars[] =
> +        "\x01\x02\x03\x04\x05\x06\x07"
> +    "\x08" /* \t \n */ "\x0B\x0C" /* \r */ "\x0E\x0F"

The alignment is off

> +    "\x10\x11\x12\x13\x14\x15\x16\x17"
> +    "\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F";
> +
> +bool
> +virStringHasControlChars(const char *str)
> +{
> +    if (!str)
> +        return false;
> +
> +    return strcspn(str, control_chars) != strlen(str);
> +}

Missing newlines between function and comment.

> +/**
> + * virStringStripControlChars:
> + * @str: the string to strip
> + *
> + * Modify the string in-place to remove the control characters
> + * in the interval: (0x01, 0x20)

The interval is more like  <0x01, 0x20).

> + */
> +void
> +virStringStripControlChars(char *str)
> +{
> +    size_t len, i, j;
> +
> +    if (!virStringHasControlChars(str))
> +        return;

The check above calls strlen and iterates through the string looking for
the characters.

> +
> +    len = strlen(str);
> +    for (i = 0, j = 0; i < len; i++) {
> +        if (index(control_chars, str[i]))
> +            continue;

Here you iterate through the string and check for the characters ... I
think it should be safe to just execute this function right away.

> +
> +        str[j++] = str[i];
> +    }
> +    str[j] = '\0';
> +}

The rest looks good.

ACK with the nits above addressed.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150415/88131be7/attachment-0001.sig>