On 5/31/23 20:50, Ján Tomko wrote:
> In virNodeDeviceGetSCSIHostCaps, there is a pattern of reusing
> a tmp value and stealing the pointer.
>
> But in two case it is not stolen. Use separate variables for them
> to avoid mixing autofree with manual free() calls.
>
> This fixes the memory leak of the "max_npiv_vports" string.
> (The other gets freed anyway because it was the last use of "tmp"
> in the function")
>
> Fixes: 8a0cb5f73ade3900546718eabe70cb064c6bd22c
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
> v3: fix "declaration after statement" warning
> src/conf/node_device_conf.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index fcee9c027c..172223225f 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -2857,29 +2857,32 @@ virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHost *scsi_host)
> }
>
> if (virVHBAIsVportCapable(NULL, scsi_host->host)) {
> + g_autofree char *max_vports = NULL;
> + g_autofree char *vports = NULL;
> +
> scsi_host->flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
>
> - if (!(tmp = virVHBAGetConfig(NULL, scsi_host->host,
> + if (!(max_vports = virVHBAGetConfig(NULL, scsi_host->host,
> "max_npiv_vports"))) {
Any reason to not align this second argument?
> VIR_WARN("Failed to read max_npiv_vports for host%d",
> scsi_host->host);
> goto cleanup;
> }
>
> - if (virStrToLong_i(tmp, NULL, 10, &scsi_host->max_vports) < 0) {
> - VIR_WARN("Failed to parse value of max_npiv_vports '%s'", tmp);
> + if (virStrToLong_i(max_vports, NULL, 10, &scsi_host->max_vports) < 0) {
> + VIR_WARN("Failed to parse value of max_npiv_vports '%s'", max_vports);
> goto cleanup;
> }
>
> - if (!(tmp = virVHBAGetConfig(NULL, scsi_host->host,
> + if (!(vports = virVHBAGetConfig(NULL, scsi_host->host,
> "npiv_vports_inuse"))) {
Same question here.
> VIR_WARN("Failed to read npiv_vports_inuse for host%d",
> scsi_host->host);
> goto cleanup;
> }
>
> - if (virStrToLong_i(tmp, NULL, 10, &scsi_host->vports) < 0) {
> - VIR_WARN("Failed to parse value of npiv_vports_inuse '%s'", tmp);
> + if (virStrToLong_i(vports, NULL, 10, &scsi_host->vports) < 0) {
> + VIR_WARN("Failed to parse value of npiv_vports_inuse '%s'", vports);
> goto cleanup;
> }
> }
Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
Michal
