On Mon, Oct 19, 2026 at 11:19:29AM -0500, Eric Blake wrote:
> 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.
g_mem_set_vtable() is deprecated and a nop. Interposing no longer works.
g_mem_is_system_malloc() always returns TRUE.
If we take advantage of glib internals, then you can call g_free() on
malloc() memory AFAIK. But I don't think the API documentation
guarantees this will always be the case and I'm not sure it's worth
baking in this assumption.
Maybe there is a clean way to do this with the glib API: define a type
called SystemMallocPtr and then use g_autoptr(SystemMallocPtr)?
If someone with more glib knowledge has a clean solution, we could start
using it. IMO if you need to think too much about it, it's probably best
to do the simple thing instead.
>
> > ---
> > 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
>
