Re: [RFC PATCH 1/2] node_device_conf: virNodeDeviceGetSCSITargetCaps: fix memory leak

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

 



On a Wednesday in 2024, Marc Hartmayer wrote:
Make sure the old value in `scsi_target->wwpn` is free'd before replacing it.
While at it, simplify the code.


"While at it" usually means it should have been a separate commit,
especially if the simplification changes the behavior of the code.

==9104== 38 bytes in 2 blocks are definitely lost in loss record 1,943 of 3,250
==9104==    at 0x483B8C0: malloc (vg_replace_malloc.c:442)
==9104==    by 0x4DFB69B: g_malloc (gmem.c:130)
==9104==    by 0x4E1921D: g_strdup (gstrfuncs.c:363)
==9104==    by 0x495D60B: g_strdup_inline (gstrfuncs.h:321)
==9104==    by 0x495D60B: virFCReadRportValue (virfcp.c:62)
==9104==    by 0x4A5F5CB: virNodeDeviceGetSCSITargetCaps (node_device_conf.c:2914)
==9104==    by 0xBF62529: udevProcessSCSITarget (node_device_udev.c:657)
==9104==    by 0xBF62529: udevGetDeviceDetails (node_device_udev.c:1406)
==9104==    by 0xBF62529: udevAddOneDevice (node_device_udev.c:1563)
==9104==    by 0xBF639B5: udevProcessDeviceListEntry (node_device_udev.c:1637)
==9104==    by 0xBF639B5: udevEnumerateDevices (node_device_udev.c:1691)
==9104==    by 0xBF639B5: nodeStateInitializeEnumerate (node_device_udev.c:2009)
==9104==    by 0x49BDBFD: virThreadHelper (virthread.c:256)
==9104==    by 0x5242069: start_thread (in /usr/lib64/libc.so.6)

Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
---
src/conf/node_device_conf.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 48140b17baa5..c146a9f0c881 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2894,9 +2894,9 @@ int
virNodeDeviceGetSCSITargetCaps(const char *sysfsPath,
                               virNodeDevCapSCSITarget *scsi_target)
{
-    int ret = -1;
    g_autofree char *dir = NULL;
    g_autofree char *rport = NULL;
+    g_autofree char *wwpn = NULL;

    VIR_DEBUG("Checking if '%s' is an FC remote port", scsi_target->name);

@@ -2906,28 +2906,21 @@ virNodeDeviceGetSCSITargetCaps(const char *sysfsPath,
    rport = g_path_get_basename(dir);

    if (!virFCIsCapableRport(rport))
-        goto cleanup;
+        return -1;
+
+    if (virFCReadRportValue(rport, "port_name",
+                            &wwpn) < 0) {
+        VIR_WARN("Failed to read port_name for '%s'", rport);
+        return -1;
+    }

    VIR_FREE(scsi_target->rport);
    scsi_target->rport = g_steal_pointer(&rport);

-    if (virFCReadRportValue(scsi_target->rport, "port_name",
-                            &scsi_target->wwpn) < 0) {
-        VIR_WARN("Failed to read port_name for '%s'", scsi_target->rport);
-        goto cleanup;
-    }
-
+    VIR_FREE(scsi_target->wwpn);
+    scsi_target->wwpn = g_steal_pointer(&wwpn);
    scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT;
-    ret = 0;
-
- cleanup:
-    if (ret < 0) {
-        VIR_FREE(scsi_target->rport);
-        VIR_FREE(scsi_target->wwpn);
-        scsi_target->flags &= ~VIR_NODE_DEV_CAP_FLAG_FC_RPORT;

Before, we cleared out the info if we were not able to refresh it.
Is there a possibility that would lead to stale information?

Jano

-    }
-
-    return ret;
+    return 0;
}


--
2.34.1
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx

[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