| Message ID | 20200908151052.713-2-luoyonggang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | rcu: fixes rcu and test-logging.c | expand |
On Tue, Sep 08, 2020 at 11:10:51PM +0800, Yonggang Luo wrote: > g_dir_make_tmp Returns the actual name used. This string should be > freed with g_free() when not needed any longer and is is in the GLib > file name encoding. In case of errors, NULL is returned and error will > be set. Use g_autofree to free it properly > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > tests/test-logging.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/test-logging.c b/tests/test-logging.c > index 8a1161de1d..957f6c08cd 100644 > --- a/tests/test-logging.c > +++ b/tests/test-logging.c > @@ -196,7 +196,7 @@ static void rmdir_full(gchar const *root) > > int main(int argc, char **argv) > { > - gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); > + g_autofree gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); > int rc; > > g_test_init(&argc, &argv, NULL); I don't see the memory leak. There is a g_free(tmp_path) at the bottom of main(). Did I miss something? Stefan
On Wed, Sep 9, 2020 at 4:30 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > On Tue, Sep 08, 2020 at 11:10:51PM +0800, Yonggang Luo wrote: > > g_dir_make_tmp Returns the actual name used. This string should be > > freed with g_free() when not needed any longer and is is in the GLib > > file name encoding. In case of errors, NULL is returned and error will > > be set. Use g_autofree to free it properly > > > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > --- > > tests/test-logging.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/test-logging.c b/tests/test-logging.c > > index 8a1161de1d..957f6c08cd 100644 > > --- a/tests/test-logging.c > > +++ b/tests/test-logging.c > > @@ -196,7 +196,7 @@ static void rmdir_full(gchar const *root) > > > > int main(int argc, char **argv) > > { > > - gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); > > + g_autofree gchar *tmp_path = > g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); > > int rc; > > > > g_test_init(&argc, &argv, NULL); > > I don't see the memory leak. There is a g_free(tmp_path) at the bottom > of main(). > > Did I miss something? > Oh, gocha, this issue fixed by someone else. So when I rebasing, something are lost. I am intent replace the free with g_autofree , should I update it? this is not a fix anymore, just a improve > > Stefan >
On Wed, Sep 09, 2020 at 04:35:26PM +0800, 罗勇刚(Yonggang Luo) wrote: > On Wed, Sep 9, 2020 at 4:30 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > On Tue, Sep 08, 2020 at 11:10:51PM +0800, Yonggang Luo wrote: > > > g_dir_make_tmp Returns the actual name used. This string should be > > > freed with g_free() when not needed any longer and is is in the GLib > > > file name encoding. In case of errors, NULL is returned and error will > > > be set. Use g_autofree to free it properly > > > > > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > --- > > > tests/test-logging.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tests/test-logging.c b/tests/test-logging.c > > > index 8a1161de1d..957f6c08cd 100644 > > > --- a/tests/test-logging.c > > > +++ b/tests/test-logging.c > > > @@ -196,7 +196,7 @@ static void rmdir_full(gchar const *root) > > > > > > int main(int argc, char **argv) > > > { > > > - gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); > > > + g_autofree gchar *tmp_path = > > g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); > > > int rc; > > > > > > g_test_init(&argc, &argv, NULL); > > > > I don't see the memory leak. There is a g_free(tmp_path) at the bottom > > of main(). > > > > Did I miss something? > > > Oh, gocha, this issue fixed by someone else. So when I rebasing, something > are lost. > I am intent replace the free with g_autofree , should I update it? this > is not a fix anymore, just > a improve If you want. It's not essential in this function since there aren't return statements where memory leaks often occur, but since you're already working on the code, it's still an improvement to use g_autofree. Stefan
diff --git a/tests/test-logging.c b/tests/test-logging.c index 8a1161de1d..957f6c08cd 100644 --- a/tests/test-logging.c +++ b/tests/test-logging.c @@ -196,7 +196,7 @@ static void rmdir_full(gchar const *root) int main(int argc, char **argv) { - gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); + g_autofree gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL); int rc; g_test_init(&argc, &argv, NULL);