Message ID | 1333729032-24441-7-git-send-email-afaerber@suse.de |
---|---|
State | New |
Headers | show |
On Fri, Apr 06, 2012 at 06:17:12PM +0200, Andreas Färber wrote: > free() opcode tables. They are being malloc()'ed in create_new_table(). This doesn't seem right. Unless I've missed something in the call path, create_new_table() is not called from the instance initializer. So surely it should not be torn down in the instance finalizer. > Resolves Jocelyn's TODO in former cpu_ppc_close(). > > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > target-ppc/translate_init.c | 14 ++++++++++++++ > 1 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index bb81bbc..5365229 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -10261,6 +10261,19 @@ static void ppc_cpu_initfn(Object *obj) > #endif /* !CONFIG_USER_ONLY */ > } > > +static void ppc_cpu_uninitfn(Object *obj) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(obj); > + CPUPPCState *env = &cpu->env; > + int i; > + > + for (i = 0; i < 0x40; i++) { > + if (env->opcodes[i] != &invalid_handler) { > + free(env->opcodes[i]); > + } > + } > +} > + > static void ppc_cpu_class_init(ObjectClass *oc, void *data) > { > PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); > @@ -10275,6 +10288,7 @@ static const TypeInfo ppc_cpu_type_info = { > .parent = TYPE_CPU, > .instance_size = sizeof(PowerPCCPU), > .instance_init = ppc_cpu_initfn, > + .instance_finalize = ppc_cpu_uninitfn, > .abstract = false, > .class_size = sizeof(PowerPCCPUClass), > .class_init = ppc_cpu_class_init,
Am 11.04.2012 03:06, schrieb David Gibson: > On Fri, Apr 06, 2012 at 06:17:12PM +0200, Andreas Färber wrote: >> free() opcode tables. They are being malloc()'ed in create_new_table(). > > This doesn't seem right. Unless I've missed something in the call > path, create_new_table() is not called from the instance initializer. > So surely it should not be torn down in the instance finalizer. We have this call chain: helper.c:cpu_ppc_init() -> translate_init.c:cpu_ppc_register_internal() -> create_ppc_opcodes() -> create_ppc_opcodes() -> { fill_new_table() (filling it with &invalid_handler), register_insn() -> { register_[dbl]ind_insn() -> register_ind_in_table() -> create_new_table(), register_direct_insn() -> insert_in_table() }(each filling it with non-invalid handlers) } So you are correct that it is not directly called from the initfn. The reason not to do that yet is that cpu_ppc_register_internal() still uses ppc_def_t, which my previous RFC patch series replaces through QOM subclasses. Since free() works fine with zero'ed memory such as after the object_new() in cpu_ppc_init() I still think this patch is fully correct. But we can postpone it if you prefer. Andreas
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index bb81bbc..5365229 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -10261,6 +10261,19 @@ static void ppc_cpu_initfn(Object *obj) #endif /* !CONFIG_USER_ONLY */ } +static void ppc_cpu_uninitfn(Object *obj) +{ + PowerPCCPU *cpu = POWERPC_CPU(obj); + CPUPPCState *env = &cpu->env; + int i; + + for (i = 0; i < 0x40; i++) { + if (env->opcodes[i] != &invalid_handler) { + free(env->opcodes[i]); + } + } +} + static void ppc_cpu_class_init(ObjectClass *oc, void *data) { PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); @@ -10275,6 +10288,7 @@ static const TypeInfo ppc_cpu_type_info = { .parent = TYPE_CPU, .instance_size = sizeof(PowerPCCPU), .instance_init = ppc_cpu_initfn, + .instance_finalize = ppc_cpu_uninitfn, .abstract = false, .class_size = sizeof(PowerPCCPUClass), .class_init = ppc_cpu_class_init,
free() opcode tables. They are being malloc()'ed in create_new_table(). Resolves Jocelyn's TODO in former cpu_ppc_close(). Signed-off-by: Andreas Färber <afaerber@suse.de> --- target-ppc/translate_init.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)