1. Patchew
  2. Libvirt
  3. View series

[PATCH] nodedevmdevctltest: Fix two memleaks

Michal Privoznik posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/f32294a4522e1167c30fcef68a828b4fde49cff8.1708950157.git.mprivozn@redhat.com
tests/nodedevmdevctltest.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
[PATCH] nodedevmdevctltest: Fix two memleaks
Posted by Michal Privoznik 2 months ago 
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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] nodedevmdevctltest: Fix two memleaks
Posted by Peter Krempa 2 months ago 
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>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] nodedevmdevctltest: Fix two memleaks
Posted by Boris Fiuczynski 2 months ago 
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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.