| Message ID | 20200226033037.18253-3-pannengyuan@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix two small memleaks | expand |
On 26.02.20 04:30, Pan Nengyuan wrote: > collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory. > It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan. > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > --- > qemu-img.c | 2 ++ > 1 file changed, 2 insertions(+) I think this should happen in the caller. And there I think it would make more sense to completely discard the old object and allocate a new one: qapi_free_ImageCheck(check); check = g_new0(ImageCheck, 1); This way, we can’t forget to free any fields if new pointers were to be added to the ImageCheck object. Max
On 2/26/2020 6:13 PM, Max Reitz wrote: > On 26.02.20 04:30, Pan Nengyuan wrote: >> collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory. >> It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan. >> >> Reported-by: Euler Robot <euler.robot@huawei.com> >> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> >> --- >> qemu-img.c | 2 ++ >> 1 file changed, 2 insertions(+) > > I think this should happen in the caller. And there I think it would > make more sense to completely discard the old object and allocate a new one: > > qapi_free_ImageCheck(check); > check = g_new0(ImageCheck, 1); > > This way, we can’t forget to free any fields if new pointers were to be > added to the ImageCheck object. Good idea, thanks. > > Max >
diff --git a/qemu-img.c b/qemu-img.c index 2b4562b9d9..bcbca6c9a2 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -638,6 +638,8 @@ static int collect_image_check(BlockDriverState *bs, return ret; } + g_free(check->filename); + g_free(check->format); check->filename = g_strdup(filename); check->format = g_strdup(bdrv_get_format_name(bs)); check->check_errors = result.check_errors;
collect_image_check() is called twice in img_check(), the filename/format will be alloced without free the original memory. It is not a big deal since the process will exit anyway, but seems like a clean code and it will remove the warning spotted by asan. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> --- qemu-img.c | 2 ++ 1 file changed, 2 insertions(+)