[libvirt PATCH v2 1/3] nodedev: fix potential leak of command
When returning early due to errors, cmd will be leaked. Use an autoptr to handle these early returns without leaking memory. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> --- src/node_device/node_device_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b0cc63ed42..e565cc29ec 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -724,7 +724,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def, char **errbuf) { g_autofree char *parent_addr = NULL; - virCommand *cmd = NULL; + g_autoptr(virCommand) cmd = NULL; const char *subcommand = virMdevctlCommandTypeToString(cmd_type); g_autofree char *inbuf = NULL; @@ -787,7 +787,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def, virCommandSetErrorBuffer(cmd, errbuf); - return cmd; + return g_steal_pointer(&cmd); } -- 2.26.3
Coverity complained that the 'default' case of the switch in nodeDeviceGetMdevctlCommand() was falling through without initializing 'cmd'. Return NULL in this case even though it should never happen. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> --- src/node_device/node_device_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index e565cc29ec..49f3cc166d 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -743,7 +743,7 @@ nodeDeviceGetMdevctlCommand(virNodeDeviceDef *def, case MDEVCTL_CMD_LAST: default: /* SHOULD NEVER HAPPEN */ - break; + return NULL; } switch (cmd_type) { -- 2.26.3
virCommandRun() already handles the case where the cmd argument is NULL, so there's no need for the caller to check. Make all callers consistent and remove unnecessary NULL checks. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/node_device/node_device_driver.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 49f3cc166d..8a0a2c3847 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -799,9 +799,6 @@ virMdevctlCreate(virNodeDeviceDef *def, char **uuid, char **errmsg) MDEVCTL_CMD_CREATE, uuid, errmsg); - if (!cmd) - return -1; - /* an auto-generated uuid is returned via stdout if no uuid is specified in * the mdevctl args */ if (virCommandRun(cmd, &status) < 0 || status != 0) @@ -822,9 +819,6 @@ virMdevctlDefine(virNodeDeviceDef *def, char **uuid, char **errmsg) MDEVCTL_CMD_DEFINE, uuid, errmsg); - if (!cmd) - return -1; - /* an auto-generated uuid is returned via stdout if no uuid is specified in * the mdevctl args */ if (virCommandRun(cmd, &status) < 0 || status != 0) -- 2.26.3
On 4/21/21 5:52 PM, Jonathon Jongsma wrote:
virCommandRun() already handles the case where the cmd argument is NULL, so there's no need for the caller to check. Make all callers consistent and remove unnecessary NULL checks.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/node_device/node_device_driver.c | 6 ------ 1 file changed, 6 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Jonathon Jongsma -
Michal Privoznik