conf: virDomainDiskSourceFormat: check for srcpool presence correctly

[libvirt PATCH] conf: virDomainDiskSourceFormat: check for srcpool presence correctly

Posted by Ján Tomko 5 months, 4 weeks ago

As a guard against programming errors, one part of the condition
only dereferences srcpool if it exists, other one does not.

Move the check up one level so that it actually has a chance to do
something useful.

Fixes: 19b1c0d31988a3a10c4694c10c27eb15c018f450
Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/conf/domain_conf.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 80f467ae7a..99f9d56f07 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22357,14 +22357,12 @@ virDomainDiskSourceFormat(virBuffer *buf,
     g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
     g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
 
-    if (src->type == VIR_STORAGE_TYPE_VOLUME) {
-        if (src->srcpool) {
-            virBufferEscapeString(&attrBuf, " pool='%s'", src->srcpool->pool);
-            virBufferEscapeString(&attrBuf, " volume='%s'", src->srcpool->volume);
-            if (src->srcpool->mode)
-                virBufferAsprintf(&attrBuf, " mode='%s'",
-                                  virStorageSourcePoolModeTypeToString(src->srcpool->mode));
-        }
+    if (src->type == VIR_STORAGE_TYPE_VOLUME && src->srcpool) {
+        virBufferEscapeString(&attrBuf, " pool='%s'", src->srcpool->pool);
+        virBufferEscapeString(&attrBuf, " volume='%s'", src->srcpool->volume);
+        if (src->srcpool->mode)
+            virBufferAsprintf(&attrBuf, " mode='%s'",
+                              virStorageSourcePoolModeTypeToString(src->srcpool->mode));
 
         if (flags & VIR_DOMAIN_DEF_FORMAT_VOLUME_TRANSLATED &&
             src->srcpool->actualtype != VIR_STORAGE_TYPE_NONE) {
-- 
2.41.0

Re: [libvirt PATCH] conf: virDomainDiskSourceFormat: check for srcpool presence correctly

Posted by Andrea Bolognani 5 months, 3 weeks ago

On Wed, Nov 01, 2023 at 03:22:12PM +0100, Ján Tomko wrote:
> As a guard against programming errors, one part of the condition
> only dereferences srcpool if it exists, other one does not.
>
> Move the check up one level so that it actually has a chance to do
> something useful.
>
> Fixes: 19b1c0d31988a3a10c4694c10c27eb15c018f450
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/conf/domain_conf.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org