On 5/4/21 1:40 PM, Kristina Hanicova wrote:
> Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 190 ++++++++++++++++++++---------------------
> 1 file changed, 94 insertions(+), 96 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5ac15fe9e8..c5b13783f3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11350,112 +11350,112 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDef *def,
> virDomainChrDef *chr_def,
> xmlXPathContextPtr ctxt)
> {
> - bool logParsed = false;
> - bool protocolParsed = false;
> - int sourceParsed = 0;
> + xmlNodePtr log = NULL;
> + xmlNodePtr protocol = NULL;
> + g_autofree xmlNodePtr *source = NULL;
> + int n = 0;
> + VIR_XPATH_NODE_AUTORESTORE(ctxt)
>
> - for (; cur; cur = cur->next) {
> - if (cur->type != XML_ELEMENT_NODE)
> - continue;
> + ctxt->node = cur;
>
> - if (virXMLNodeNameEqual(cur, "source")) {
> - /* Parse only the first source element since only one is used
> - * for chardev devices, the only exception is UDP type, where
> - * user can specify two source elements. */
> - if (sourceParsed >= 1 && def->type != VIR_DOMAIN_CHR_TYPE_UDP) {
> - virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("only one source element is allowed for "
> - "character device"));
> - goto error;
> - } else if (sourceParsed >= 2) {
> - virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("only two source elements are allowed for "
> - "character device"));
> - goto error;
> - }
> - sourceParsed++;
> + if ((n = virXPathNodeSet("./source", ctxt, &source)) > 0) {
> + /* Parse only the first source element since only one is used
> + * for chardev devices, the only exception is UDP type, where
> + * user can specify two source elements. */
> + if (n > 1 && def->type != VIR_DOMAIN_CHR_TYPE_UDP) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("only one source element is allowed for "
> + "character device"));
> + goto error;
> + } else if (n > 2) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("only two source elements are allowed for "
> + "character device"));
> + goto error;
> + }
>
> - switch ((virDomainChrType) def->type) {
> - case VIR_DOMAIN_CHR_TYPE_FILE:
> - if (virDomainChrSourceDefParseFile(def, cur) < 0)
> - goto error;
> - break;
> + switch ((virDomainChrType) def->type) {
> + case VIR_DOMAIN_CHR_TYPE_FILE:
> + if (virDomainChrSourceDefParseFile(def, source[0]) < 0)
> + goto error;
> + break;
>
> - case VIR_DOMAIN_CHR_TYPE_PTY:
> - /* PTY path is only parsed from live xml. */
> - if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
> - def->data.file.path = virXMLPropString(cur, "path");
> - break;
> + case VIR_DOMAIN_CHR_TYPE_PTY:
> + /* PTY path is only parsed from live xml. */
> + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
> + def->data.file.path = virXPathString("string(./source/@path)", ctxt);
> + break;
>
> - case VIR_DOMAIN_CHR_TYPE_DEV:
> - case VIR_DOMAIN_CHR_TYPE_PIPE:
> - def->data.file.path = virXMLPropString(cur, "path");
> - break;
> + case VIR_DOMAIN_CHR_TYPE_DEV:
> + case VIR_DOMAIN_CHR_TYPE_PIPE:
> + def->data.file.path = virXPathString("string(./source/@path)", ctxt);
> + break;
>
> - case VIR_DOMAIN_CHR_TYPE_UNIX:
> - if (virDomainChrSourceDefParseUnix(def, cur, ctxt) < 0)
> - goto error;
> - break;
> + case VIR_DOMAIN_CHR_TYPE_UNIX:
> + if (virDomainChrSourceDefParseUnix(def, source[0], ctxt) < 0)
> + goto error;
> + break;
>
> - case VIR_DOMAIN_CHR_TYPE_UDP:
> - if (virDomainChrSourceDefParseUDP(def, cur) < 0)
> + case VIR_DOMAIN_CHR_TYPE_UDP:
> + if ((virDomainChrSourceDefParseUDP(def, source[0]) < 0) ||
> + (source[1] && virDomainChrSourceDefParseUDP(def, source[1]) < 0))
Ooops. While this may work by chance, it's not correct.
virXPathNodeSet() does not return a NULL terminated array. Therefore, if
n == 1, then source[0] is allocated, but source[1] is not. Fortunately,
the fix is as easy as s/source[1]/n == 2/.
> goto error;
> - break;
> + break;
Michal
