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] Cc: [email protected] Signed-off-by: Stefan Hajnoczi <[email protected]> --- v2: - Use g_autofree instead of manual free(3) calls [Eric] --- scsi/qemu-pr-helper.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index 54d196a503f..1f97a5c4bfe 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -559,7 +559,7 @@ static char *get_dm_name(int fd, Error **errp) static bool multipathd_cmd(int fd, const char *cmd, Error **errp) { const unsigned int timeout_ms = 60000; - char *reply = NULL; + g_autofree char *reply = NULL; int last_errno; int ret; @@ -568,11 +568,16 @@ static bool multipathd_cmd(int fd, const char *cmd, Error **errp) if (reply && strcmp(reply, "ok\n") != 0) { error_setg(errp, "mpath_process_cmd(%d, \"%s\") replied \"%s\"", fd, cmd, reply); - free(reply); return false; } - free(reply); if (ret == -1) { + /* + * It should already be NULL but the mpath_process_cmd() documentation + * only defines it when the function returns success, so don't depend + * on its value when ret == -1. + */ + reply = NULL; + error_setg_errno(errp, last_errno, "mpath_process_cmd(%d, \"%s\") failed", fd, cmd); -- 2.56.0
