[Qemu-devel] [RFC PATCH v1 1/6] migration: Fix unregister_savevm()

Bharata B Rao posted 6 patches 8 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [RFC PATCH v1 1/6] migration: Fix unregister_savevm()
Posted by Bharata B Rao 8 years, 8 months ago
In unregister_savevm(), free se->compat only if it was allocated earlier.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 migration/savevm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 352a8f2..7a268ec 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -648,7 +648,9 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
     QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
         if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
             QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
-            g_free(se->compat);
+            if (dev) {
+                g_free(se->compat);
+            }
             g_free(se->ops);
             g_free(se);
         }
-- 
2.7.4


Re: [Qemu-devel] [RFC PATCH v1 1/6] migration: Fix unregister_savevm()
Posted by David Gibson 8 years, 8 months ago
On Wed, May 17, 2017 at 09:19:17AM +0530, Bharata B Rao wrote:
> In unregister_savevm(), free se->compat only if it was allocated earlier.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

I don't think this is necessary.  If se->compat was never allocated,
then it should be NULL (since se is allocated with g_new0()).
g_free() is explicitly safe to call on NULL, and we already rely on
that in qemu.

> ---
>  migration/savevm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 352a8f2..7a268ec 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -648,7 +648,9 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
>      QTAILQ_FOREACH_SAFE(se, &savevm_state.handlers, entry, new_se) {
>          if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
>              QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
> -            g_free(se->compat);
> +            if (dev) {
> +                g_free(se->compat);
> +            }
>              g_free(se->ops);
>              g_free(se);
>          }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [RFC PATCH v1 1/6] migration: Fix unregister_savevm()
Posted by Bharata B Rao 8 years, 8 months ago
On Wed, May 17, 2017 at 04:43:00PM +1000, David Gibson wrote:
> On Wed, May 17, 2017 at 09:19:17AM +0530, Bharata B Rao wrote:
> > In unregister_savevm(), free se->compat only if it was allocated earlier.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> I don't think this is necessary.  If se->compat was never allocated,
> then it should be NULL (since se is allocated with g_new0()).
> g_free() is explicitly safe to call on NULL, and we already rely on
> that in qemu.

Yeah, this is not necessary, will get rid of this in the next iteration.

Regards.,
Bharata.