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
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
