[PATCH] nodedevmdevctltest: Fix two memleaks
There are two memleaks inside of nodedevmdevctltest: 1) In the testCommandDryRunCallback() - when appending lines to stdinbuf the pointer is overwritten without freeing the old memory it pointed to. 2) In testMdevctlModify() the livecmd variable is reused and since its marked as g_autoptr() the first use leaks. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/nodedevmdevctltest.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index de688c982e..827036fa74 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -33,10 +33,13 @@ testCommandDryRunCallback(const char *const*args G_GNUC_UNUSED, { char **stdinbuf = opaque; - if (*stdinbuf) - *stdinbuf = g_strconcat(*stdinbuf, "\n", input, NULL); - else + if (*stdinbuf) { + char *new = g_strconcat(*stdinbuf, "\n", input, NULL); + VIR_FREE(*stdinbuf); + *stdinbuf = g_steal_pointer(&new); + } else { *stdinbuf = g_strdup(input); + } } typedef virCommand * (*MdevctlCmdFunc)(virNodeDeviceDef *, char **, char **); @@ -188,6 +191,7 @@ testMdevctlModify(const void *data G_GNUC_UNUSED) int ret = -1; g_autoptr(virCommand) definedcmd = NULL; g_autoptr(virCommand) livecmd = NULL; + g_autoptr(virCommand) livecmd_update = NULL; g_autoptr(virCommand) bothcmd = NULL; g_autofree char *errmsg = NULL; g_autofree char *stdinbuf = NULL; @@ -222,10 +226,10 @@ testMdevctlModify(const void *data G_GNUC_UNUSED) &parser_callbacks, NULL, false))) goto cleanup; - if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def_update, false, true, &errmsg))) + if (!(livecmd_update = nodeDeviceGetMdevctlModifyCommand(def_update, false, true, &errmsg))) goto cleanup; - if (virCommandRun(livecmd, NULL) < 0) + if (virCommandRun(livecmd_update, NULL) < 0) goto cleanup; if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def, false, true, &errmsg))) -- 2.43.0
Michal, thanks for fixing this. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 2/26/24 13:22, Michal Privoznik wrote:
There are two memleaks inside of nodedevmdevctltest:
1) In the testCommandDryRunCallback() - when appending lines to stdinbuf the pointer is overwritten without freeing the old memory it pointed to.
2) In testMdevctlModify() the livecmd variable is reused and since its marked as g_autoptr() the first use leaks.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/nodedevmdevctltest.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index de688c982e..827036fa74 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -33,10 +33,13 @@ testCommandDryRunCallback(const char *const*args G_GNUC_UNUSED, { char **stdinbuf = opaque;
- if (*stdinbuf) - *stdinbuf = g_strconcat(*stdinbuf, "\n", input, NULL); - else + if (*stdinbuf) { + char *new = g_strconcat(*stdinbuf, "\n", input, NULL); + VIR_FREE(*stdinbuf); + *stdinbuf = g_steal_pointer(&new); + } else { *stdinbuf = g_strdup(input); + } }
typedef virCommand * (*MdevctlCmdFunc)(virNodeDeviceDef *, char **, char **); @@ -188,6 +191,7 @@ testMdevctlModify(const void *data G_GNUC_UNUSED) int ret = -1; g_autoptr(virCommand) definedcmd = NULL; g_autoptr(virCommand) livecmd = NULL; + g_autoptr(virCommand) livecmd_update = NULL; g_autoptr(virCommand) bothcmd = NULL; g_autofree char *errmsg = NULL; g_autofree char *stdinbuf = NULL; @@ -222,10 +226,10 @@ testMdevctlModify(const void *data G_GNUC_UNUSED) &parser_callbacks, NULL, false))) goto cleanup;
- if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def_update, false, true, &errmsg))) + if (!(livecmd_update = nodeDeviceGetMdevctlModifyCommand(def_update, false, true, &errmsg))) goto cleanup;
- if (virCommandRun(livecmd, NULL) < 0) + if (virCommandRun(livecmd_update, NULL) < 0) goto cleanup;
if (!(livecmd = nodeDeviceGetMdevctlModifyCommand(def, false, true, &errmsg)))
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon, Feb 26, 2024 at 13:22:37 +0100, Michal Privoznik wrote:
There are two memleaks inside of nodedevmdevctltest:
1) In the testCommandDryRunCallback() - when appending lines to stdinbuf the pointer is overwritten without freeing the old memory it pointed to.
2) In testMdevctlModify() the livecmd variable is reused and since its marked as g_autoptr() the first use leaks.
Please add the "Fixes" tag:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/nodedevmdevctltest.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index de688c982e..827036fa74 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -33,10 +33,13 @@ testCommandDryRunCallback(const char *const*args G_GNUC_UNUSED, { char **stdinbuf = opaque;
- if (*stdinbuf) - *stdinbuf = g_strconcat(*stdinbuf, "\n", input, NULL); - else + if (*stdinbuf) {
Alternatively: g_autofree char *oldbuf = g_steal_pointer(stdinbuf); *stdinbuf = g_strconcat(oldbuf, "\n", input, NULL);
+ char *new = g_strconcat(*stdinbuf, "\n", input, NULL); + VIR_FREE(*stdinbuf); + *stdinbuf = g_steal_pointer(&new); + } else { *stdinbuf = g_strdup(input); + } }
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (3)
-
Boris Fiuczynski -
Michal Privoznik -
Peter Krempa