Re: [PATCH 3/3] conf: Rework virDomainSEVDefParseXML()

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

 



On Wed, Jun 13, 2018 at 12:51:59PM +0200, Michal Privoznik wrote:
Firstly, this function changes node for relative XPaths but
doesn't restore the original one in case VIR_ALLOC(def) fails.

This should not matter, since we're going to abort parsing anyway.

Secondly, @type is leaked. Thirdly, dh-cert and session

s/dh-cert/dhCert

It has been renamed in the meantime.

attributes are strdup()-ed needlessly, virXPathString already
does that so we can use the retval immediately.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/conf/domain_conf.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 85f07af46e..c788b525b5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15849,17 +15849,16 @@ static virDomainSevDefPtr
virDomainSEVDefParseXML(xmlNodePtr sevNode,
                        xmlXPathContextPtr ctxt)
{
-    char *tmp = NULL;
    char *type = NULL;
    xmlNodePtr save = ctxt->node;
    virDomainSevDefPtr def;
    unsigned long policy;

+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
    ctxt->node = sevNode;

-    if (VIR_ALLOC(def) < 0)
-        return NULL;
-

Or just 'goto error' instead of moving the allocation.
Not sure which option is more future-proof.

    if (!(type = virXMLPropString(sevNode, "type"))) {
        virReportError(VIR_ERR_XML_ERROR, "%s",
                       _("missing launch-security type"));
@@ -15899,29 +15898,18 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
    }

    def->policy = policy;
+    def->dh_cert = virXPathString("string(./dh-cert)", ctxt);

string(./dhCert)

+    def->session = virXPathString("string(./session)", ctxt);

-    if ((tmp = virXPathString("string(./dh-cert)", ctxt))) {
-        if (VIR_STRDUP(def->dh_cert, tmp) < 0)
-            goto error;
-
-        VIR_FREE(tmp);
-    }
-

With the dh-cert -> dhCert adjustments:

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[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