diff mbox

[3/7] tpm-backend: Initialize and free data members in it's own methods

Message ID 1490965817-16913-4-git-send-email-amarnath.valluri@intel.com
State New
Headers show

Commit Message

Valluri, Amarnath March 31, 2017, 1:10 p.m. UTC
Initialize and free TPMBackend data members in it's own instance_init() and
instance_finalize methods.

Took the opportunity to fix object cleanup in tpm_backend_destroy() method

Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
 backends/tpm.c           | 14 +++++++++++++-
 hw/tpm/tpm_passthrough.c |  8 +-------
 2 files changed, 14 insertions(+), 8 deletions(-)

Comments

Marc-André Lureau April 4, 2017, 12:57 p.m. UTC | #1
Hi

On Fri, Mar 31, 2017 at 5:01 PM Amarnath Valluri <amarnath.valluri@intel.com>
wrote:

> Initialize and free TPMBackend data members in it's own instance_init() and
> instance_finalize methods.
>
> Took the opportunity to fix object cleanup in tpm_backend_destroy() method
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
>  backends/tpm.c           | 14 +++++++++++++-
>  hw/tpm/tpm_passthrough.c |  8 +-------
>  2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 50a7809..00c82d7 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -59,7 +59,7 @@ void tpm_backend_destroy(TPMBackend *s)
>
>      k->ops->destroy(s);
>
> -    tpm_backend_thread_end(s);
> +    object_unref(OBJECT(s));
>  }
>

tpm_backend_destroy() wrapping object_unref().. I think we could do it the
correct way instead, caller use object_unref() and implementation can use
finalizers.

>
>  int tpm_backend_init(TPMBackend *s, TPMState *state,
> @@ -187,10 +187,20 @@ static void tpm_backend_prop_set_opened(Object *obj,
> bool value, Error **errp)
>
>  static void tpm_backend_instance_init(Object *obj)
>  {
> +    TPMBackend *s = TPM_BACKEND(obj);
> +
>      object_property_add_bool(obj, "opened",
>                               tpm_backend_prop_get_opened,
>                               tpm_backend_prop_set_opened,
>                               NULL);
> +    s->id = NULL;
> +    s->fe_model = -1;
> +    s->opened = false;
> +    s->tpm_state = NULL;
> +    s->thread_pool = NULL;
> +    s->recv_data_callback = NULL;
> +    s->path = NULL;
> +    s->cancel_path = NULL;
>

I don't see much point in initializing again those fields to 0, since
object creation do it already.

>
>  }
>
> @@ -199,6 +209,8 @@ static void tpm_backend_instance_finalize(Object *obj)
>      TPMBackend *s = TPM_BACKEND(obj);
>
>      g_free(s->id);
> +    g_free(s->path);
> +    g_free(s->cancel_path);
>      tpm_backend_thread_end(s);
>  }
>
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 606e064..cb63079 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -418,8 +418,6 @@ static TPMBackend *tpm_passthrough_create(QemuOpts
> *opts, const char *id)
>      TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>
>      tb->id = g_strdup(id);
> -    /* let frontend set the fe_model to proper value */
> -    tb->fe_model = -1;
>
>      if (tpm_passthrough_handle_device_opts(opts, tb)) {
>          goto err_exit;
> @@ -433,7 +431,7 @@ static TPMBackend *tpm_passthrough_create(QemuOpts
> *opts, const char *id)
>      return tb;
>
>  err_exit:
> -    g_free(tb->id);
> +    object_unref(obj);
>

ok, that's a leak fix


>
>      return NULL;
>  }
> @@ -446,10 +444,6 @@ static void tpm_passthrough_destroy(TPMBackend *tb)
>
>      qemu_close(tpm_pt->tpm_fd);
>      qemu_close(tpm_pt->cancel_fd);
> -
> -    g_free(tb->id);
> -    g_free(tb->path);
> -    g_free(tb->cancel_path);
>      g_free(tpm_pt->tpm_dev);
>  }
>
> --
> 2.7.4
>
>
> --
Marc-André Lureau
diff mbox

Patch

diff --git a/backends/tpm.c b/backends/tpm.c
index 50a7809..00c82d7 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -59,7 +59,7 @@  void tpm_backend_destroy(TPMBackend *s)
 
     k->ops->destroy(s);
 
-    tpm_backend_thread_end(s);
+    object_unref(OBJECT(s));
 }
 
 int tpm_backend_init(TPMBackend *s, TPMState *state,
@@ -187,10 +187,20 @@  static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp)
 
 static void tpm_backend_instance_init(Object *obj)
 {
+    TPMBackend *s = TPM_BACKEND(obj);
+
     object_property_add_bool(obj, "opened",
                              tpm_backend_prop_get_opened,
                              tpm_backend_prop_set_opened,
                              NULL);
+    s->id = NULL;
+    s->fe_model = -1;
+    s->opened = false;
+    s->tpm_state = NULL;
+    s->thread_pool = NULL;
+    s->recv_data_callback = NULL;
+    s->path = NULL;
+    s->cancel_path = NULL;
 
 }
 
@@ -199,6 +209,8 @@  static void tpm_backend_instance_finalize(Object *obj)
     TPMBackend *s = TPM_BACKEND(obj);
 
     g_free(s->id);
+    g_free(s->path);
+    g_free(s->cancel_path);
     tpm_backend_thread_end(s);
 }
 
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 606e064..cb63079 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -418,8 +418,6 @@  static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
 
     tb->id = g_strdup(id);
-    /* let frontend set the fe_model to proper value */
-    tb->fe_model = -1;
 
     if (tpm_passthrough_handle_device_opts(opts, tb)) {
         goto err_exit;
@@ -433,7 +431,7 @@  static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
     return tb;
 
 err_exit:
-    g_free(tb->id);
+    object_unref(obj);
 
     return NULL;
 }
@@ -446,10 +444,6 @@  static void tpm_passthrough_destroy(TPMBackend *tb)
 
     qemu_close(tpm_pt->tpm_fd);
     qemu_close(tpm_pt->cancel_fd);
-
-    g_free(tb->id);
-    g_free(tb->path);
-    g_free(tb->cancel_path);
     g_free(tpm_pt->tpm_dev);
 }