On 10/10/2016 04:15 PM, Sławek Kapłoński wrote:
This new function can be used to check if e.g. name of XML node don't
s/don't/doesn't/
contains forbidden chars like "/" or new-line.
s/new-line/'\n'
--- src/util/virxml.c | 18 ++++++++++++++++++ src/util/virxml.h | 4 ++++ 2 files changed, 22 insertions(+)
Couple of other nits... There are a few other string manipulation API's in src/util/virstring.c whether these changes belong there is a matter of interpretation. Certainly there's quite a few places where you might also get some ideas especially with respect to handling these types of things from other parts of libvirt... If you went with a common virstring function, it could check check and return the 0 or -1 allowing/forcing the caller to provide the error message...
diff --git a/src/util/virxml.c b/src/util/virxml.c index 03bd784..450487e 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -890,6 +890,24 @@ virXMLChildElementCount(xmlNodePtr node) return ret; }
We've been using 2 blank lines between functions lately.
+/** + * virXMLCheckString: checks if string contains at least one of + * forbidden characters
More recent comments would be: * virXMLCheckString: * @nodename: <description> * @str: <description> * @forbiddenChars: <description> * * <Function description> *
+ * + * Returns: 0 if string don't contains any of given characters, -1 otherwise
s/don't/doesn't
+ */ +int virXMLCheckString(const char *nodeName, char *str, + const char *forbiddenChars)
We've been using: int virXMLCheckString(const char *nodeName, char *str, const char *forbiddenChars) I would think 'str' is const too since you're not changing it.
+{ + char *c; + c = strpbrk(str, forbiddenChars); + if (c) {
if ((c = strpbrk(str, forbiddenChars))) {
+ virReportError(VIR_ERR_XML_DETAIL, + _("invalid char in %s: %c"), nodeName, *c);
The _( needs to be lined up below the VIR, e.g. virReportError(VIR_ERR_XML_DETAIL, _("invalid char in ..."), ...); [the ... args could be on the second line] Now if the *c is '\n' (or any other non-printable), then printing won't do much good without some form of escaping. Alternatively the hex value could be printed... You may want to look at {virStringHas|virStringStrip}ControlChars too for some more thoughts.
+ return -1; + } + return 0; +}
/** * virXMLNodeToString: convert an XML node ptr to an XML string diff --git a/src/util/virxml.h b/src/util/virxml.h index 7a0a1da..676bf5e 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -75,6 +75,10 @@ char * virXMLPropString(xmlNodePtr node, const char *name); long virXMLChildElementCount(xmlNodePtr node);
+int virXMLCheckString(const char *nodeName, + char *str, + const char *forbiddenChars);
I see you're just following existing practices in the module that's good. John
+ /* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, const char *filename,