[v1] domain_conf: Fix memory leak when free'ing virDomainObj

[PATCH] domain_conf: Fix memory leak when free'ing virDomainObj

Posted by Luke Yue 2 years, 8 months ago

Free dom->deprecations when free'ing virDomainObj

Signed-off-by: Luke Yue <lukedyue@gmail.com>
---
 src/conf/domain_conf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 09da4ab952..2527cd1d70 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3733,6 +3733,7 @@ void virDomainDefFree(virDomainDef *def)
 static void virDomainObjDispose(void *obj)
 {
     virDomainObj *dom = obj;
+    size_t i = 0;
 
     VIR_DEBUG("obj=%p", dom);
     virCondDestroy(&dom->cond);
@@ -3742,6 +3743,11 @@ static void virDomainObjDispose(void *obj)
     if (dom->privateDataFreeFunc)
         (dom->privateDataFreeFunc)(dom->privateData);
 
+    for (i = 0; i < dom->ndeprecations; i++) {
+        g_free(dom->deprecations[i]);
+    }
+    g_free(dom->deprecations);
+
     virDomainSnapshotObjListFree(dom->snapshots);
     virDomainCheckpointObjListFree(dom->checkpoints);
 }
-- 
2.32.0

Re: [PATCH] domain_conf: Fix memory leak when free'ing virDomainObj

Posted by Pavel Hrdina 2 years, 8 months ago

On Mon, Aug 09, 2021 at 03:04:35PM +0800, Luke Yue wrote:
> Free dom->deprecations when free'ing virDomainObj
> 
> Signed-off-by: Luke Yue <lukedyue@gmail.com>
> ---
>  src/conf/domain_conf.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 09da4ab952..2527cd1d70 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3733,6 +3733,7 @@ void virDomainDefFree(virDomainDef *def)
>  static void virDomainObjDispose(void *obj)
>  {
>      virDomainObj *dom = obj;
> +    size_t i = 0;
>  
>      VIR_DEBUG("obj=%p", dom);
>      virCondDestroy(&dom->cond);
> @@ -3742,6 +3743,11 @@ static void virDomainObjDispose(void *obj)
>      if (dom->privateDataFreeFunc)
>          (dom->privateDataFreeFunc)(dom->privateData);
>  
> +    for (i = 0; i < dom->ndeprecations; i++) {
> +        g_free(dom->deprecations[i]);
> +    }
> +    g_free(dom->deprecations);
> +

Nice catch, can you add the code to separate function? For example:

virDomainObjDepracationFree()

Otherwise looks good.

Pavel

>      virDomainSnapshotObjListFree(dom->snapshots);
>      virDomainCheckpointObjListFree(dom->checkpoints);
>  }
> -- 
> 2.32.0
>

Re: [PATCH] domain_conf: Fix memory leak when free'ing virDomainObj

Posted by Luke Yue 2 years, 8 months ago

On Tue, 2021-08-10 at 11:51 +0200, Pavel Hrdina wrote:
> On Mon, Aug 09, 2021 at 03:04:35PM +0800, Luke Yue wrote:
> > Free dom->deprecations when free'ing virDomainObj
> > 
> > Signed-off-by: Luke Yue <lukedyue@gmail.com>
> > ---
> >  src/conf/domain_conf.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 09da4ab952..2527cd1d70 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -3733,6 +3733,7 @@ void virDomainDefFree(virDomainDef *def)
> >  static void virDomainObjDispose(void *obj)
> >  {
> >      virDomainObj *dom = obj;
> > +    size_t i = 0;
> >  
> >      VIR_DEBUG("obj=%p", dom);
> >      virCondDestroy(&dom->cond);
> > @@ -3742,6 +3743,11 @@ static void virDomainObjDispose(void *obj)
> >      if (dom->privateDataFreeFunc)
> >          (dom->privateDataFreeFunc)(dom->privateData);
> >  
> > +    for (i = 0; i < dom->ndeprecations; i++) {
> > +        g_free(dom->deprecations[i]);
> > +    }
> > +    g_free(dom->deprecations);
> > +
> 
> Nice catch, can you add the code to separate function? For example:
> 
> virDomainObjDepracationFree()
> 
> Otherwise looks good.
> 
> Pavel
> 
> >      virDomainSnapshotObjListFree(dom->snapshots);
> >      virDomainCheckpointObjListFree(dom->checkpoints);
> >  }
> > -- 
> > 2.32.0
> > 

Thanks for the review! Will send v2 soon.