Re: [PATCH] qemu-pr-helper: do not free reply when multipath_process_cmd() fails

[Eric Blake <eblake@redhat.com>]

On Mon, Oct 19, 2026 at 11:33:53AM -0400, Stefan Hajnoczi wrote:
> Kevin Wolf pointed out that the multipath_process_cmd() documentation
> only defines the value of reply on success. Don't touch it when the
> function returns -1.
> 
> The libmpathcmd implementation as of c78cf64a9e8e does not set *reply to
> NULL when its internal mpath_send_cmd() call fails, so I think Kevin's
> interpretation is correct.
> 
> As it stands, there is no observable bug in the QEMU code though because
> reply is initialized to NULL before calling multipath_process_cmd().
> This patch is just a cleanup.
> 
> Cc: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: kwolf@redhat.com
> Cc: eblake@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> Eric pointed out that g_autofree could eliminate the manual free(3)
> calls. In this case we cannot use it since g_autofree calls glib's
> g_free() rather than the standard library's free(3).

I know glib encourages g_free() in case malloc was interposed, but
aren't we guaranteed that we are not exploiting glib's malloc
overrides, and therefore g_free() calls into free(3)?  But I'm fine
with your decision to not use g_autofree here.

> ---
>  scsi/qemu-pr-helper.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
> index 54d196a503f..d52109cab25 100644
> --- a/scsi/qemu-pr-helper.c
> +++ b/scsi/qemu-pr-helper.c
> @@ -571,13 +571,17 @@ static bool multipathd_cmd(int fd, const char *cmd, Error **errp)
>          free(reply);
>          return false;
>      }
> -    free(reply);
>      if (ret == -1) {
> +        /*
> +         * reply is not freed because the documentation only defines it when
> +         * the function returns success.
> +         */
>          error_setg_errno(errp, last_errno,
>                           "mpath_process_cmd(%d, \"%s\") failed",
>                           fd, cmd);
>          return false;
>      }
> +    free(reply);

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org