[PATCH] block vvfat.c fix leak when failure occurs

Daniella Lee posted 1 patch 2 years, 5 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20211119112553.352222-1-daniellalee111@gmail.com
Maintainers: Hanna Reitz <hreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
block/vvfat.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
[PATCH] block vvfat.c fix leak when failure occurs
Posted by Daniella Lee 2 years, 5 months ago
Based on your suggestions. I made a new patch which contians:
1.format detection
2.replace calloc with g_malloc0 in enable_write_target function
3.use g_free without null pointer detection in vvfat_open function
4.delete line "ret = 0", use return ret directly in vvfat_open function


Signed-off-by: Daniella Lee <daniellalee111@gmail.com>
---
 block/vvfat.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 05e78e3c27..5dacc6cfac 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1279,8 +1279,18 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
 
     qemu_co_mutex_init(&s->lock);
 
-    ret = 0;
+    qemu_opts_del(opts);
+
+    return 0;
+
 fail:
+    g_free(s->qcow_filename);
+    s->qcow_filename = NULL;
+    g_free(s->cluster_buffer);
+    s->cluster_buffer = NULL;
+    g_free(s->used_clusters);
+    s->used_clusters = NULL;
+
     qemu_opts_del(opts);
     return ret;
 }
@@ -3118,7 +3128,7 @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
     int size = sector2cluster(s, s->sector_count);
     QDict *options;
 
-    s->used_clusters = calloc(size, 1);
+    s->used_clusters = g_malloc0(size);
 
     array_init(&(s->commits), sizeof(commit_t));
 
@@ -3166,8 +3176,6 @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
     return 0;
 
 err:
-    g_free(s->qcow_filename);
-    s->qcow_filename = NULL;
     return ret;
 }
 
-- 
2.17.1


Re: [PATCH] block vvfat.c fix leak when failure occurs
Posted by Hanna Reitz 2 years, 5 months ago
On 19.11.21 12:25, Daniella Lee wrote:
> Based on your suggestions. I made a new patch which contians:
> 1.format detection
> 2.replace calloc with g_malloc0 in enable_write_target function
> 3.use g_free without null pointer detection in vvfat_open function
> 4.delete line "ret = 0", use return ret directly in vvfat_open function
>
>
> Signed-off-by: Daniella Lee <daniellalee111@gmail.com>
> ---
>   block/vvfat.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)

Thanks, looks good to me!

Two remarks for the next time: When you send a new version of a patch, 
you should mark it as “v2” (and then “v3” and so on) in the subject, 
e.g. “[PATCH v2]”.  This can be done e.g. with `git format-patch -v2`.

Second, for a new iteration, you should generally keep the commit 
message from the previous one and not replace it with a change log. 
Having a change log is very nice, don’t get me wrong, but it shouldn’t 
be part of the commit message.  Once you’ve formatted the patch with 
`git format-patch`, you can edit the file and put the change log below 
the “---” line, e.g. like it was done here: 
https://lists.nongnu.org/archive/html/qemu-block/2021-09/msg00453.html

No need for you to respin this patch, though, I’ve just replaced the 
commit message with the one from v1 and applied it to my block branch:

https://gitlab.com/hreitz/qemu/-/commits/block/

Thanks again,

Hanna