Re: [libvirt] [PATCH 1/2] util: Add function to check if string contains some chars

On 14.10.2016 04:53, Sławek Kapłoński wrote: > This new function can be used to check if e.g. name of XML node > don't contains forbidden chars like "/" or new-line. > --- > src/conf/network_conf.c | 2 +- > src/libvirt_private.syms | 1 + > src/util/virstring.c | 9 +++++++++ > src/util/virstring.h | 1 + > 4 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c > index aa39776..949d138 100644 > --- a/src/conf/network_conf.c > +++ b/src/conf/network_conf.c > @@ -2117,7 +2117,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) > goto error; > } > > - if (strchr(def->name, '/')) { > + if (virStringHasChars(def->name, "/")) { > virReportError(VIR_ERR_XML_ERROR, > _("name %s cannot contain '/'"), def->name); Usually, in one patch we introduce a function and then in a subsequent one we switch the rest of the code into using it. But okay, I can live with this too. > goto error; > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 55b6a24..6f41234 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -2435,6 +2435,7 @@ virStringEncodeBase64; > virStringFreeList; > virStringFreeListCount; > virStringGetFirstWithPrefix; > +virStringHasChars; > virStringHasControlChars; > virStringIsEmpty; > virStringIsPrintable; > diff --git a/src/util/virstring.c b/src/util/virstring.c > index 4a70620..7799d87 100644 > --- a/src/util/virstring.c > +++ b/src/util/virstring.c > @@ -975,6 +975,15 @@ virStringStripIPv6Brackets(char *str) > } > > > +bool > +virStringHasChars(const char *str, const char *chars) > +{ > + if (strpbrk(str, chars)) > + return true; > + return false; > +} This, however, makes not much sense. I mean, this function has no added value to pain strpbrk(). What I suggested in one of previous reviews was that this function should report an error too. In that case, it will immediately have added value and thus it will be handy to use it. Perhaps you are afraid that error message might change in some cases? That's okay, we don't make any promises about error messages.

> + > + > static const char control_chars[] = > "\x01\x02\x03\x04\x05\x06\x07" > "\x08" /* \t \n */ "\x0B\x0C" /* \r */ "\x0E\x0F" > diff --git a/src/util/virstring.h b/src/util/virstring.h > index 8854d5f..7f2c96d 100644 > --- a/src/util/virstring.h > +++ b/src/util/virstring.h > @@ -272,6 +272,7 @@ char *virStringReplace(const char *haystack, > ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); > > void virStringStripIPv6Brackets(char *str); > +bool virStringHasChars(const char *str, const char *chars); > bool virStringHasControlChars(const char *str); > void virStringStripControlChars(char *str); > > Michal