Re: [Qemu-devel] [PATCH v2 3/9] tpm-backend: Initialize and free data members in it's own methods

[Stefan Berger]

On 04/07/2017 10:30 AM, Amarnath Valluri 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_{create,destroy}
> methods
>
> Signed-off-by: Amarnath Valluri <address@hidden>
> ---
>   backends/tpm.c           |  8 ++++++--
>   hw/tpm/tpm_passthrough.c | 18 +++++++++++-------
>   2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 0ec0c67..99d2b2b 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -57,7 +57,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,
> @@ -182,11 +182,13 @@ 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->fe_model = -1;
>   }
>
>   static void tpm_backend_instance_finalize(Object *obj)
> @@ -194,6 +196,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 f50d9cf..65831b5 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -417,8 +417,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;
> @@ -432,7 +430,7 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, 
> const char *id)
>       return tb;
>
>   err_exit:
> -    g_free(tb->id);
> +    object_unref(obj);
>
>       return NULL;
>   }
> @@ -445,10 +443,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);


Looks like here you are correcting the error from the previous patch...



>       g_free(tpm_pt->tpm_dev);
>   }
>
> @@ -486,10 +480,20 @@ static const TPMDriverOps tpm_passthrough_driver = {
>
>   static void tpm_passthrough_inst_init(Object *obj)
>   {
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
> +
> +    tpm_pt->tpm_fd = -1;
> +    tpm_pt->cancel_fd = -1;
>   }
>
>   static void tpm_passthrough_inst_finalize(Object *obj)
>   {
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
> +
> +    tpm_passthrough_cancel_cmd(TPM_BACKEND(obj));
> +
> +    qemu_close(tpm_pt->tpm_fd);
> +    qemu_close(tpm_pt->cancel_fd);
>   }

With these I am wondering whether they should not have been moved here 
from tpm_passthrough_destroy()? The appear there as well.

>   static void tpm_passthrough_class_init(ObjectClass *klass, void *data)