Removed code is unreacheble, since the following functions always return 0:
    virCPUx86DataAdd()
    libxlCapsAddCPUID()
    virCapabilitiesAddHostFeature()
    libxl_get_physinfo()
    virCapabilitiesSetNetPrefix()
    libxlMakeDomainOSCaps()
    libxlMakeDomainDeviceDiskCaps()
    libxlMakeDomainDeviceGraphicsCaps()
    libxlMakeDomainDeviceVideoCaps()

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
---
 src/libxl/libxl_capabilities.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 177e8b988e..68908874fb 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -64,12 +64,7 @@ libxlCapsAddCPUID(virCPUData *data, virCPUx86CPUID *cpuid, ssize_t ncaps)
     item.type = VIR_CPU_X86_DATA_CPUID;
     for (i = 0; i < ncaps; i++) {
         item.data.cpuid = cpuid[i];
-
-        if (virCPUx86DataAdd(data, &item) < 0) {
-            VIR_DEBUG("Failed to add CPUID(%x,%x)",
-                      cpuid[i].eax_in, cpuid[i].ecx_in);
-            return -1;
-        }
+        virCPUx86DataAdd(data, &item) < 0);
     }
 
     return 0;
@@ -119,8 +114,7 @@ libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap)
         return NULL;
 
     ncaps = G_N_ELEMENTS(cpuid);
-    if (libxlCapsAddCPUID(cpudata, cpuid, ncaps) < 0)
-        return NULL;
+    libxlCapsAddCPUID(cpudata, cpuid, ncaps);
 
     return g_steal_pointer(&cpudata);
 }
@@ -145,9 +139,10 @@ libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info)
     cpu = virCPUDefNew();
 
     host_pae = phy_info->hw_cap[0] & LIBXL_X86_FEATURE_PAE_MASK;
-    if (host_pae &&
-        virCapabilitiesAddHostFeature(caps, "pae") < 0)
+    if (host_pae) {
+        virCapabilitiesAddHostFeature(caps, "pae");
         return -1;
+    }
 
     host_lm = (phy_info->hw_cap[2] & LIBXL_X86_FEATURE_LM_MASK);
     if (host_lm)
@@ -179,17 +174,12 @@ libxlCapsInitHost(libxl_ctx *ctx, virCaps *caps)
     int ret = -1;
 
     libxl_physinfo_init(&phy_info);
-    if (libxl_get_physinfo(ctx, &phy_info) != 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Failed to get node physical info from libxenlight"));
-        goto cleanup;
-    }
+    libxl_get_physinfo(ctx, &phy_info);
 
     if (libxlCapsInitCPU(caps, &phy_info) < 0)
         goto cleanup;
 
-    if (virCapabilitiesSetNetPrefix(caps, LIBXL_GENERATED_PREFIX_XEN) < 0)
-        goto cleanup;
+    virCapabilitiesSetNetPrefix(caps, LIBXL_GENERATED_PREFIX_XEN);
 
     ret = 0;
 
@@ -685,11 +675,11 @@ libxlMakeDomainCapabilities(virDomainCaps *domCaps,
     else
         domCaps->maxvcpus = PV_MAX_VCPUS;
 
-    if (libxlMakeDomainOSCaps(domCaps->machine, os, firmwares, nfirmwares) < 0 ||
-        libxlMakeDomainDeviceDiskCaps(disk) < 0 ||
-        libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 ||
-        libxlMakeDomainDeviceVideoCaps(video) < 0)
-        return -1;
+    libxlMakeDomainOSCaps(domCaps->machine, os, firmwares, nfirmwares);
+    libxlMakeDomainDeviceDiskCaps(disk);
+    libxlMakeDomainDeviceGraphicsCaps(graphics);
+    libxlMakeDomainDeviceVideoCaps(video);
+
     if (STRNEQ(domCaps->machine, "xenpvh") &&
         libxlMakeDomainDeviceHostdevCaps(hostdev) < 0)
         return -1;
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On 12/13/23 11:56, Dmitry Frolov wrote:
> Removed code is unreacheble, since the following functions always return 0:
>     virCPUx86DataAdd()
>     libxlCapsAddCPUID()
>     virCapabilitiesAddHostFeature()
>     libxl_get_physinfo()
>     virCapabilitiesSetNetPrefix()
>     libxlMakeDomainOSCaps()
>     libxlMakeDomainDeviceDiskCaps()
>     libxlMakeDomainDeviceGraphicsCaps()
>     libxlMakeDomainDeviceVideoCaps()
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
> ---
>  src/libxl/libxl_capabilities.c | 34 ++++++++++++----------------------
>  1 file changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
> index 177e8b988e..68908874fb 100644
> --- a/src/libxl/libxl_capabilities.c
> +++ b/src/libxl/libxl_capabilities.c
> @@ -64,12 +64,7 @@ libxlCapsAddCPUID(virCPUData *data, virCPUx86CPUID *cpuid, ssize_t ncaps)
>      item.type = VIR_CPU_X86_DATA_CPUID;
>      for (i = 0; i < ncaps; i++) {
>          item.data.cpuid = cpuid[i];
> -
> -        if (virCPUx86DataAdd(data, &item) < 0) {
> -            VIR_DEBUG("Failed to add CPUID(%x,%x)",
> -                      cpuid[i].eax_in, cpuid[i].ecx_in);
> -            return -1;
> -        }
> +        virCPUx86DataAdd(data, &item) < 0);

This will not compile.

I am not against this change, but to make it even more complete, we
should turn those functions into void. I mean, virCPUx86DataAdd()
returns nothing else than 0? Perfect! Let's make it return nothing then.

And it can be done in batches. From the list of functions the tool
identified, each function can be 'fixed' in a separate patch. For
instance, virCPUx86DataAdd() can be turned to void and all places where
its retval is checked can be fixed too. This is going to be one patch.
the second then fixes another function and places where it's called, and
so on.

Surely, libxl is not the only place where virCPUx86DataAdd() is called
and where it has its retval checked.

Michal
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org