Re: [PATCH v2 02/19] Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/4/21 1:39 PM, Kristina Hanicova wrote:
> Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx>
> ---
>  src/conf/domain_conf.c | 130 +++++++++++++++++++----------------------
>  1 file changed, 60 insertions(+), 70 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e073644810..5c5b4ad6d7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6709,14 +6709,16 @@ virDomainDeviceInfoParseXML(virDomainXMLOption *xmlopt,
>  
>  static int
>  virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
> -                                     xmlXPathContextPtr ctxt G_GNUC_UNUSED,
> +                                     xmlXPathContextPtr ctxt,
>                                       virDomainHostdevDef *def)
>  {
>      bool got_product, got_vendor;
> -    xmlNodePtr cur;
>      virDomainHostdevSubsysUSB *usbsrc = &def->source.subsys.u.usb;
>      g_autofree char *startupPolicy = NULL;
>      g_autofree char *autoAddress = NULL;
> +    VIR_XPATH_NODE_AUTORESTORE(ctxt)
> +
> +    ctxt->node = node;
>  
>      if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
>          def->startupPolicy =
> @@ -6737,79 +6739,67 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
>      got_product = false;
>      got_vendor = false;
>  
> -    cur = node->children;
> -    while (cur != NULL) {
> -        if (cur->type == XML_ELEMENT_NODE) {
> -            if (virXMLNodeNameEqual(cur, "vendor")) {
> -                g_autofree char *vendor = virXMLPropString(cur, "id");
> -
> -                if (vendor) {
> -                    got_vendor = true;
> -                    if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0) {
> -                        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                       _("cannot parse vendor id %s"), vendor);
> -                        return -1;
> -                    }
> -                } else {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   "%s", _("usb vendor needs id"));
> -                    return -1;
> -                }
> -            } else if (virXMLNodeNameEqual(cur, "product")) {
> -                g_autofree char *product = virXMLPropString(cur, "id");
> -
> -                if (product) {
> -                    got_product = true;
> -                    if (virStrToLong_ui(product, NULL, 0,
> -                                        &usbsrc->product) < 0) {
> -                        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                       _("cannot parse product %s"),
> -                                       product);
> -                        return -1;
> -                    }
> -                } else {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   "%s", _("usb product needs id"));
> -                    return -1;
> -                }
> -            } else if (virXMLNodeNameEqual(cur, "address")) {
> -                g_autofree char *bus = NULL;
> -                g_autofree char *device = NULL;
> -
> -                bus = virXMLPropString(cur, "bus");
> -                if (bus) {
> -                    if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) {
> -                        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                       _("cannot parse bus %s"), bus);
> -                        return -1;
> -                    }
> -                } else {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   "%s", _("usb address needs bus id"));
> -                    return -1;
> -                }
> +    if (virXPathNode("./vendor", ctxt)) {
> +        g_autofree char *vendor = NULL;
>  
> -                device = virXMLPropString(cur, "device");
> -                if (device) {
> -                    if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0) {
> -                        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                       _("cannot parse device %s"),
> -                                       device);
> -                        return -1;
> -                    }
> -                } else {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                                   _("usb address needs device id"));
> -                    return -1;
> -                }
> -            } else {
> +        if ((vendor = virXPathString("string(./vendor/@id)", ctxt))) {
> +            got_vendor = true;
> +            if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0) {

Since you are touching this strToint conversion anyway, you could have
used virXMLPropUInt(). Nothing critical, just pointing that out. It also
applies to the rest of patches. I'll fix them before merge.

>                  virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("unknown usb source type '%s'"),
> -                               cur->name);
> +                               _("cannot parse vendor id %s"), vendor);
>                  return -1;
>              }
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("usb vendor needs id"));
> +            return -1;
> +        }
> +    }
> +
> +    if (virXPathNode("./product", ctxt)) {
> +        g_autofree char *product = NULL;
> +
> +        if ((product = virXPathString("string(./product/@id)", ctxt))) {
> +            got_product = true;
> +            if (virStrToLong_ui(product, NULL, 0, &usbsrc->product) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("cannot parse product %s"), product);
> +                return -1;
> +            }
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("usb product needs id"));
> +            return -1;
> +        }
> +    }
> +
> +    if (virXPathNode("./address", ctxt)) {
> +        g_autofree char *bus = NULL;
> +        g_autofree char *device = NULL;
> +
> +        if ((bus = virXPathString("string(./address/@bus)", ctxt))) {
> +            if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("cannot parse bus %s"), bus);
> +                return -1;
> +            }
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("usb address needs bus id"));
> +            return -1;
> +        }
> +
> +        if ((device = virXPathString("string(./address/@device)", ctxt))) {
> +            if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("cannot parse device %s"), device);
> +                return -1;
> +            }
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("usb address needs device id"));
> +            return -1;
>          }
> -        cur = cur->next;
>      }
>  
>      if (got_vendor && usbsrc->vendor == 0) {
> 


Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux