On Mar 18 23:41, Philippe Mathieu-Daudé wrote:
> On 03/16/2018 09:31 PM, Aaron Lindsay wrote:
> > Signed-off-by: Aaron Lindsay <alind...@codeaurora.org>
> > ---
> > target/arm/cpu.c | 15 ++++++++++-----
> > target/arm/cpu.h | 23 ++++++++++++-----------
> > target/arm/internals.h | 7 ++++---
> > 3 files changed, 26 insertions(+), 19 deletions(-)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 072cbbf..5f782bf 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -55,13 +55,16 @@ static bool arm_cpu_has_work(CPUState *cs)
> > | CPU_INTERRUPT_EXITTB);
> > }
> >
> > -void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHook *hook,
> > +void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
> > void *opaque)
> > {
> > - /* We currently only support registering a single hook function */
> > - assert(!cpu->el_change_hook);
> > - cpu->el_change_hook = hook;
> > - cpu->el_change_hook_opaque = opaque;
> > + ARMELChangeHook *entry;
> > + entry = g_malloc0(sizeof (*entry));
>
> imho g_malloc() is enough.
It seems like the only difference is between initializing it to zero
(g_malloc0) and making it as uninitialized (g_malloc) for coverity. Are
there coding standards for when we should choose which?
>
> > +
> > + entry->hook = hook;
> > + entry->opaque = opaque;
> > +
> > + QLIST_INSERT_HEAD(&cpu->el_change_hooks, entry, node);
> > }
> >
> > static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
> > @@ -744,6 +747,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> > **errp)
> > return;
> > }
> >
> > + QLIST_INIT(&cpu->el_change_hooks);
> > +
>
> You missed to fill arm_cpu_unrealizefn() with:
>
> QLIST_FOREACH(...) {
> QLIST_REMOVE(...);
> g_free(...);
> }
Do you mean arm_cpu_finalizefn()?
-Aaron
--
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
