[PATCH 04/12] virSecurityLabelDefParseXML: Don't reuse temporary string 'p'

Peter Krempa posted 12 patches 4 years, 2 months ago
[PATCH 04/12] virSecurityLabelDefParseXML: Don't reuse temporary string 'p'
Posted by Peter Krempa 4 years, 2 months ago
Use separate variables for 'model' and 'relabel' properties.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c | 20 ++++++++------------
 src/util/virseclabel.h |  1 +
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24de57005c..df0d033d0b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7800,15 +7800,15 @@ static virSecurityLabelDef *
 virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
                             unsigned int flags)
 {
-    char *p;
+    g_autofree char *model = NULL;
+    g_autofree char *relabel = NULL;
     virSecurityLabelDef *seclabel = NULL;

-    p = virXMLPropStringLimit(ctxt->node, "model",
-                              VIR_SECURITY_MODEL_BUFLEN - 1);
+    model = virXMLPropStringLimit(ctxt->node, "model",
+                                  VIR_SECURITY_MODEL_BUFLEN - 1);

-    if (!(seclabel = virSecurityLabelDefNew(p)))
+    if (!(seclabel = virSecurityLabelDefNew(model)))
         goto error;
-    VIR_FREE(p);

     /* set default value */
     seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
@@ -7823,16 +7823,13 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
         seclabel->type == VIR_DOMAIN_SECLABEL_NONE)
         seclabel->relabel = false;

-    VIR_FREE(p);
-    p = virXMLPropString(ctxt->node, "relabel");
-    if (p) {
-        if (virStringParseYesNo(p, &seclabel->relabel) < 0) {
+    if ((relabel = virXMLPropString(ctxt->node, "relabel"))) {
+        if (virStringParseYesNo(relabel, &seclabel->relabel) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
-                           _("invalid security relabel value %s"), p);
+                           _("invalid security relabel value '%s'"), relabel);
             goto error;
         }
     }
-    VIR_FREE(p);

     if (seclabel->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
         !seclabel->relabel) {
@@ -7905,7 +7902,6 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
     return seclabel;

  error:
-    VIR_FREE(p);
     virSecurityLabelDefFree(seclabel);
     return NULL;
 }
diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
index eca4d09d24..7e62f8a2e2 100644
--- a/src/util/virseclabel.h
+++ b/src/util/virseclabel.h
@@ -43,6 +43,7 @@ struct _virSecurityLabelDef {
 };


+
 /* Security configuration for device */
 typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef;
 struct _virSecurityDeviceLabelDef {
-- 
2.31.1

Re: [PATCH 04/12] virSecurityLabelDefParseXML: Don't reuse temporary string 'p'
Posted by Tim Wiederhake 4 years, 2 months ago
On Mon, 2021-11-22 at 18:12 +0100, Peter Krempa wrote:
> (...)
> diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
> index eca4d09d24..7e62f8a2e2 100644
> --- a/src/util/virseclabel.h
> +++ b/src/util/virseclabel.h
> @@ -43,6 +43,7 @@ struct _virSecurityLabelDef {
>  };
> 
> 
> +
>  /* Security configuration for device */
>  typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef;
>  struct _virSecurityDeviceLabelDef {

I don't think this change was intentional

Re: [PATCH 04/12] virSecurityLabelDefParseXML: Don't reuse temporary string 'p'
Posted by Ján Tomko 4 years, 2 months ago
On a Monday in 2021, Peter Krempa wrote:
>Use separate variables for 'model' and 'relabel' properties.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/domain_conf.c | 20 ++++++++------------
> src/util/virseclabel.h |  1 +
> 2 files changed, 9 insertions(+), 12 deletions(-)
>
>diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
>index eca4d09d24..7e62f8a2e2 100644
>--- a/src/util/virseclabel.h
>+++ b/src/util/virseclabel.h
>@@ -43,6 +43,7 @@ struct _virSecurityLabelDef {
> };
>
>
>+

Without this suspicious empty line:

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

> /* Security configuration for device */
> typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef;
> struct _virSecurityDeviceLabelDef {
>-- 
>2.31.1
>