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 <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Stefan Hajnoczi <[email protected]>
> ---
> 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 <[email protected]>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
