| Message ID | 1491575431-32170-7-git-send-email-amarnath.valluri@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/07/2017 10:30 AM, Amarnath Valluri wrote: > As TPMBackend is a Qemu Object, we can use object_unref() inplace of > tpm_backend_destroy() to free the backend object, hence removed destroy() from > TPMDriverOps interface. > > Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com> > --- > backends/tpm.c | 11 ----------- > hw/tpm/tpm_passthrough.c | 14 -------------- > include/sysemu/tpm_backend.h | 7 ------- > tpm.c | 2 +- > 4 files changed, 1 insertion(+), 33 deletions(-) > > diff --git a/backends/tpm.c b/backends/tpm.c > index c96f462..3493df6 100644 > --- a/backends/tpm.c > +++ b/backends/tpm.c > @@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s) > return k->ops->desc ? k->ops->desc() : ""; > } > > -void tpm_backend_destroy(TPMBackend *s) > -{ > - TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); > - > - if (k->ops->destroy) { > - k->ops->destroy(s); > - } > - > - object_unref(OBJECT(s)); > -} > - > int tpm_backend_init(TPMBackend *s, TPMState *state, > TPMRecvDataCB *datacb) > { > diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c > index 71bdf25..8e11ed3 100644 > --- a/hw/tpm/tpm_passthrough.c > +++ b/hw/tpm/tpm_passthrough.c > @@ -428,19 +428,6 @@ err_exit: > return NULL; > } > > -static void tpm_passthrough_destroy(TPMBackend *tb) > -{ > - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); > - > - tpm_passthrough_cancel_cmd(tb); > - > - qemu_close(tpm_pt->tpm_fd); > - qemu_close(tpm_pt->cancel_fd); > - g_free(tpm_pt->tpm_dev); This g_free should be in the finalizer as well, right? It isn't. So we will leak some memory. > - > - qapi_free_TPMPassthroughOptions(tpm_pt->ops); > -} > - > static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb) > { > TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); > @@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = { > .opts = tpm_passthrough_cmdline_opts, > .desc = tpm_passthrough_create_desc, > .create = tpm_passthrough_create, > - .destroy = tpm_passthrough_destroy, > .realloc_buffer = tpm_passthrough_realloc_buffer, > .reset = tpm_passthrough_reset, > .had_startup_error = tpm_passthrough_get_startup_error, > diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h > index 7f4d621..8f8f133 100644 > --- a/include/sysemu/tpm_backend.h > +++ b/include/sysemu/tpm_backend.h > @@ -77,7 +77,6 @@ struct TPMDriverOps { > const char *(*desc)(void); > > TPMBackend *(*create)(QemuOpts *opts, const char *id); > - void (*destroy)(TPMBackend *t); > > /* initialize the backend */ > int (*init)(TPMBackend *t); > @@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s); > const char *tpm_backend_get_desc(TPMBackend *s); > > /** > - * tpm_backend_destroy: > - * @s: the backend to destroy > - */ > -void tpm_backend_destroy(TPMBackend *s); > - > -/** > * tpm_backend_init: > * @s: the backend to initialized > * @state: TPMState > diff --git a/tpm.c b/tpm.c > index 1b6b550..43d980e 100644 > --- a/tpm.c > +++ b/tpm.c > @@ -197,7 +197,7 @@ void tpm_cleanup(void) > > QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) { > QLIST_REMOVE(drv, list); > - tpm_backend_destroy(drv); > + object_unref(OBJECT(drv)); > } > } > I *think* you should apply this patch after introducing the tpm_passthrough_inst_finalize or merge it into that one.
On 04/25/2017 09:59 PM, Stefan Berger wrote: > On 04/07/2017 10:30 AM, Amarnath Valluri wrote: >> As TPMBackend is a Qemu Object, we can use object_unref() inplace of >> tpm_backend_destroy() to free the backend object, hence removed >> destroy() from >> TPMDriverOps interface. >> >> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com> >> --- >> backends/tpm.c | 11 ----------- >> hw/tpm/tpm_passthrough.c | 14 -------------- >> include/sysemu/tpm_backend.h | 7 ------- >> tpm.c | 2 +- >> 4 files changed, 1 insertion(+), 33 deletions(-) >> >> diff --git a/backends/tpm.c b/backends/tpm.c >> index c96f462..3493df6 100644 >> --- a/backends/tpm.c >> +++ b/backends/tpm.c >> @@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s) >> return k->ops->desc ? k->ops->desc() : ""; >> } >> >> -void tpm_backend_destroy(TPMBackend *s) >> -{ >> - TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); >> - >> - if (k->ops->destroy) { >> - k->ops->destroy(s); >> - } >> - >> - object_unref(OBJECT(s)); >> -} >> - >> int tpm_backend_init(TPMBackend *s, TPMState *state, >> TPMRecvDataCB *datacb) >> { >> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c >> index 71bdf25..8e11ed3 100644 >> --- a/hw/tpm/tpm_passthrough.c >> +++ b/hw/tpm/tpm_passthrough.c >> @@ -428,19 +428,6 @@ err_exit: >> return NULL; >> } >> >> -static void tpm_passthrough_destroy(TPMBackend *tb) >> -{ >> - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); >> - >> - tpm_passthrough_cancel_cmd(tb); >> - >> - qemu_close(tpm_pt->tpm_fd); >> - qemu_close(tpm_pt->cancel_fd); >> - g_free(tpm_pt->tpm_dev); > > This g_free should be in the finalizer as well, right? It isn't. So we > will leak some memory. Yes, I will fix this. > > >> - >> - qapi_free_TPMPassthroughOptions(tpm_pt->ops); >> -} >> - >> static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb) >> { >> TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); >> @@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = { >> .opts = tpm_passthrough_cmdline_opts, >> .desc = tpm_passthrough_create_desc, >> .create = tpm_passthrough_create, >> - .destroy = tpm_passthrough_destroy, >> .realloc_buffer = tpm_passthrough_realloc_buffer, >> .reset = tpm_passthrough_reset, >> .had_startup_error = tpm_passthrough_get_startup_error, >> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h >> index 7f4d621..8f8f133 100644 >> --- a/include/sysemu/tpm_backend.h >> +++ b/include/sysemu/tpm_backend.h >> @@ -77,7 +77,6 @@ struct TPMDriverOps { >> const char *(*desc)(void); >> >> TPMBackend *(*create)(QemuOpts *opts, const char *id); >> - void (*destroy)(TPMBackend *t); >> >> /* initialize the backend */ >> int (*init)(TPMBackend *t); >> @@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s); >> const char *tpm_backend_get_desc(TPMBackend *s); >> >> /** >> - * tpm_backend_destroy: >> - * @s: the backend to destroy >> - */ >> -void tpm_backend_destroy(TPMBackend *s); >> - >> -/** >> * tpm_backend_init: >> * @s: the backend to initialized >> * @state: TPMState >> diff --git a/tpm.c b/tpm.c >> index 1b6b550..43d980e 100644 >> --- a/tpm.c >> +++ b/tpm.c >> @@ -197,7 +197,7 @@ void tpm_cleanup(void) >> >> QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) { >> QLIST_REMOVE(drv, list); >> - tpm_backend_destroy(drv); >> + object_unref(OBJECT(drv)); >> } >> } >> > > I *think* you should apply this patch after introducing the > tpm_passthrough_inst_finalize or merge it into that one. > Ok, i even think it makes sense to meld this commit with baeb9df(tpm-backend: Initialize and free data members in it's own methods). - Amarnath
diff --git a/backends/tpm.c b/backends/tpm.c index c96f462..3493df6 100644 --- a/backends/tpm.c +++ b/backends/tpm.c @@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s) return k->ops->desc ? k->ops->desc() : ""; } -void tpm_backend_destroy(TPMBackend *s) -{ - TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); - - if (k->ops->destroy) { - k->ops->destroy(s); - } - - object_unref(OBJECT(s)); -} - int tpm_backend_init(TPMBackend *s, TPMState *state, TPMRecvDataCB *datacb) { diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 71bdf25..8e11ed3 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -428,19 +428,6 @@ err_exit: return NULL; } -static void tpm_passthrough_destroy(TPMBackend *tb) -{ - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); - - tpm_passthrough_cancel_cmd(tb); - - qemu_close(tpm_pt->tpm_fd); - qemu_close(tpm_pt->cancel_fd); - g_free(tpm_pt->tpm_dev); - - qapi_free_TPMPassthroughOptions(tpm_pt->ops); -} - static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb) { TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); @@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = { .opts = tpm_passthrough_cmdline_opts, .desc = tpm_passthrough_create_desc, .create = tpm_passthrough_create, - .destroy = tpm_passthrough_destroy, .realloc_buffer = tpm_passthrough_realloc_buffer, .reset = tpm_passthrough_reset, .had_startup_error = tpm_passthrough_get_startup_error, diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h index 7f4d621..8f8f133 100644 --- a/include/sysemu/tpm_backend.h +++ b/include/sysemu/tpm_backend.h @@ -77,7 +77,6 @@ struct TPMDriverOps { const char *(*desc)(void); TPMBackend *(*create)(QemuOpts *opts, const char *id); - void (*destroy)(TPMBackend *t); /* initialize the backend */ int (*init)(TPMBackend *t); @@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s); const char *tpm_backend_get_desc(TPMBackend *s); /** - * tpm_backend_destroy: - * @s: the backend to destroy - */ -void tpm_backend_destroy(TPMBackend *s); - -/** * tpm_backend_init: * @s: the backend to initialized * @state: TPMState diff --git a/tpm.c b/tpm.c index 1b6b550..43d980e 100644 --- a/tpm.c +++ b/tpm.c @@ -197,7 +197,7 @@ void tpm_cleanup(void) QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) { QLIST_REMOVE(drv, list); - tpm_backend_destroy(drv); + object_unref(OBJECT(drv)); } }
As TPMBackend is a Qemu Object, we can use object_unref() inplace of tpm_backend_destroy() to free the backend object, hence removed destroy() from TPMDriverOps interface. Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com> --- backends/tpm.c | 11 ----------- hw/tpm/tpm_passthrough.c | 14 -------------- include/sysemu/tpm_backend.h | 7 ------- tpm.c | 2 +- 4 files changed, 1 insertion(+), 33 deletions(-)