Re: [PATCH] spapr: Fix Coverity warning while validating nvdimm options

[Philippe Mathieu-Daudé]

Le mer. 26 févr. 2020 13:11, Shivaprasad G Bhat <address@hidden> a écrit :

Fixes Coverity issue,
      CID 1419883:  Error handling issues  (CHECKED_RETURN)
           Calling "qemu_uuid_parse" without checking return value

nvdimm_set_uuid() already verifies if the user provided uuid is valid or
not. So, need to check for the validity during pre-plug validation again.

As this a false positive in this case, assert if not valid to be safe.

Reported-by: Coverity (CID 1419883)
Signed-off-by: Shivaprasad G Bhat <address@hidden>
---
 hw/ppc/spapr_nvdimm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index 74eeb8bb74..051727536e 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -44,7 +44,7 @@ void spapr_nvdimm_validate_opts(NVDIMMDevice *nvdimm, uint64_t size,
     }

     uuidstr = object_property_get_str(OBJECT(nvdimm), NVDIMM_UUID_PROP, NULL);
-    qemu_uuid_parse(uuidstr, &uuid);
+    g_assert(qemu_uuid_parse(uuidstr, &uuid) == 0);

From https://developer.gnome.org/glib/stable/glib-Testing.html#g-assert

The macro can be turned off in final releases of code by defining G_DISABLE_ASSERT when compiling the application, so code must not depend on any side effects from expr.

This looks like bad pattern example. 

     g_free(uuidstr);

     if (qemu_uuid_is_null(&uuid)) {