[libvirt] [PATCH] conf: use virDomainDeviceDefFree free dev
In function virDomainDeviceDefParse, we shoud use virDomainDeviceDefFree free data structure avoid potential memory leak. Signed-off-by: Xu Yandong <xuyandong2@huawei.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 848c831330..8fb9480827 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16504,7 +16504,8 @@ virDomainDeviceDefParse(const char *xmlStr, return dev; error: - VIR_FREE(dev); + virDomainDeviceDefFree(dev); + dev = NULL; goto cleanup; } -- 2.18.1
On 9/19/19 5:01 AM, Xu Yandong wrote:
In function virDomainDeviceDefParse, we shoud use virDomainDeviceDefFree free data structure avoid potential memory leak.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 848c831330..8fb9480827 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16504,7 +16504,8 @@ virDomainDeviceDefParse(const char *xmlStr, return dev;
error: - VIR_FREE(dev); + virDomainDeviceDefFree(dev); + dev = NULL;
You don't need the 'dev = NULL' after virDomainDeviceDefFree(dev). This function will end up calling VIR_FREE(dev) in the end, which will make dev = NULL. WIthout the extra dev = NULL assignment: Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
goto cleanup; }
On 9/19/19 5:01 AM, Xu Yandong wrote:
In function virDomainDeviceDefParse, we shoud use virDomainDeviceDefFree free data structure avoid potential memory leak.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 848c831330..8fb9480827 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16504,7 +16504,8 @@ virDomainDeviceDefParse(const char *xmlStr, return dev;
error: - VIR_FREE(dev); + virDomainDeviceDefFree(dev); + dev = NULL; goto cleanup;
Just noticed that the current master code does not have this potential leak anymore. It was fixed by commit 475777c9ec, introducing VIR_AUTOFREE() for virDomainDeviceDefPtr. Thanks, DHB
}
On Thu, Sep 19, 2019 at 10:33:53 -0300, Daniel Henrique Barboza wrote:
On 9/19/19 5:01 AM, Xu Yandong wrote:
In function virDomainDeviceDefParse, we shoud use virDomainDeviceDefFree free data structure avoid potential memory leak.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 848c831330..8fb9480827 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16504,7 +16504,8 @@ virDomainDeviceDefParse(const char *xmlStr, return dev; error: - VIR_FREE(dev); + virDomainDeviceDefFree(dev); + dev = NULL; goto cleanup;
Just noticed that the current master code does not have this potential leak anymore. It was fixed by commit 475777c9ec, introducing VIR_AUTOFREE() for virDomainDeviceDefPtr.
VIR_AUTOFREE calls only a VIR_FREE to release the memory associated with 'dev', so if this patch fixes a leak of one of the members of the 'dev' a patch is still required with a different approach. E.g. VIR_AUTOPTR.
On Thu, Sep 19, 2019 at 05:23:26PM +0200, Peter Krempa wrote:
On Thu, Sep 19, 2019 at 10:33:53 -0300, Daniel Henrique Barboza wrote:
On 9/19/19 5:01 AM, Xu Yandong wrote:
In function virDomainDeviceDefParse, we shoud use virDomainDeviceDefFree free data structure avoid potential memory leak.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 848c831330..8fb9480827 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16504,7 +16504,8 @@ virDomainDeviceDefParse(const char *xmlStr, return dev; error: - VIR_FREE(dev); + virDomainDeviceDefFree(dev); + dev = NULL; goto cleanup;
Just noticed that the current master code does not have this potential leak anymore. It was fixed by commit 475777c9ec, introducing VIR_AUTOFREE() for virDomainDeviceDefPtr.
VIR_AUTOFREE calls only a VIR_FREE to release the memory associated with 'dev', so if this patch fixes a leak of one of the members of the 'dev' a patch is still required with a different approach. E.g. VIR_AUTOPTR.
Pushed now: commit 3f40a487a9a820004214574f82f0e492a836adf0 conf: use correct free function for virDomainDeviceDef Jano
participants (4)
-
Daniel Henrique Barboza -
Ján Tomko -
Peter Krempa -
Xu Yandong