Re: [for-5.0 PATCH v2 1/3] cpu: Introduce CPUReset callback typedef

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 10 Dec 2019 at 10:39, Markus Armbruster <armbru@redhat.com> wrote:
>
> Greg Kurz <groug@kaod.org> writes:
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 77c6f0529903..047e3972ecaf 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -74,6 +74,8 @@ typedef struct CPUWatchpoint CPUWatchpoint;
> >
> >  struct TranslationBlock;
> >
> > +typedef void (*CPUReset)(CPUState *cpu);
> > +
> >  /**
> >   * CPUClass:
> >   * @class_by_name: Callback to map -cpu command line model name to an
> > @@ -165,7 +167,7 @@ typedef struct CPUClass {
> >      ObjectClass *(*class_by_name)(const char *cpu_model);
> >      void (*parse_features)(const char *typename, char *str, Error **errp);
> >
> > -    void (*reset)(CPUState *cpu);
> > +    CPUReset reset;
> >      int reset_dump_flags;
> >      bool (*has_work)(CPUState *cpu);
> >      void (*do_interrupt)(CPUState *cpu);
> [...]
>
> Opinion, not objection: such typedefs make the code less obvious.

It's particularly odd here where this class has several
methods but we've only chosen one to privilege with a typedef.

Personal preference: if you use a typedef, typedef the
function type, not the pointer-to-the-function-type.
But I would just leave it be.

thanks
-- PMM