[PATCH] cutils: Fix memleak in get_relocated_path()

Zhenzhong Duan posted 1 patch 3 years ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210426223042.119554-1-zhenzhong.duan@intel.com
util/cutils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] cutils: Fix memleak in get_relocated_path()
Posted by Zhenzhong Duan 3 years ago
Valgrind complains definitely loss in get_relocated_path(), because
GString is leaked in get_relocated_path() when returning with gchar *.
Use g_string_free(, false) to free GString while preserving gchar *.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 util/cutils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/cutils.c b/util/cutils.c
index ee908486da..f58c2157d2 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir)
         assert(G_IS_DIR_SEPARATOR(dir[-1]));
         g_string_append(result, dir - 1);
     }
-    return result->str;
+    return g_string_free(result, FALSE);
 }
-- 
2.25.1


Re: [PATCH] cutils: Fix memleak in get_relocated_path()
Posted by Philippe Mathieu-Daudé 3 years ago
Hi,

On 4/27/21 12:30 AM, Zhenzhong Duan wrote:
> Valgrind complains definitely loss in get_relocated_path(), because
> GString is leaked in get_relocated_path() when returning with gchar *.
> Use g_string_free(, false) to free GString while preserving gchar *.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  util/cutils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/cutils.c b/util/cutils.c
> index ee908486da..f58c2157d2 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir)
>          assert(G_IS_DIR_SEPARATOR(dir[-1]));
>          g_string_append(result, dir - 1);
>      }
> -    return result->str;
> +    return g_string_free(result, FALSE);
>  }
> 

Thanks for your patch, but Stefano sent the same fix 2 weeks ago:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg798279.html

It should be merged once the development tree opens again (we are
now 'freezed' before the v6.0.0 release).

You might want to send your Review-by or Tested-by tag to Stefano's
patch.

Regards,

Phil.


RE: [PATCH] cutils: Fix memleak in get_relocated_path()
Posted by Duan, Zhenzhong 3 years ago
> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Monday, April 26, 2021 10:06 PM
> To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; qemu-
> devel@nongnu.org
> Cc: qemu-trivial@nongnu.org; pbonzini@redhat.com; Stefano Garzarella
> <sgarzare@redhat.com>
> Subject: Re: [PATCH] cutils: Fix memleak in get_relocated_path()
> 
> Hi,
> 
> On 4/27/21 12:30 AM, Zhenzhong Duan wrote:
> > Valgrind complains definitely loss in get_relocated_path(), because
> > GString is leaked in get_relocated_path() when returning with gchar *.
> > Use g_string_free(, false) to free GString while preserving gchar *.
> >
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > ---
> >  util/cutils.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/util/cutils.c b/util/cutils.c index
> > ee908486da..f58c2157d2 100644
> > --- a/util/cutils.c
> > +++ b/util/cutils.c
> > @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir)
> >          assert(G_IS_DIR_SEPARATOR(dir[-1]));
> >          g_string_append(result, dir - 1);
> >      }
> > -    return result->str;
> > +    return g_string_free(result, FALSE);
> >  }
> >
> 
> Thanks for your patch, but Stefano sent the same fix 2 weeks ago:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg798279.html
> 
> It should be merged once the development tree opens again (we are now
> 'freezed' before the v6.0.0 release).

I see, thanks for your quick response.

Zhenzhong