Re: [XEN PATCH 1/5] xen/common: address violations of MISRA C:2012 Rule 8.2

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 19 Nov 2023, Federico Serafini wrote:
> On 18/11/23 03:59, Stefano Stabellini wrote:
> > On Fri, 17 Nov 2023, Federico Serafini wrote:
> > > diff --git a/xen/common/stop_machine.c b/xen/common/stop_machine.c
> > > index 3adbe380de..398cfd507c 100644
> > > --- a/xen/common/stop_machine.c
> > > +++ b/xen/common/stop_machine.c
> > > @@ -46,7 +46,7 @@ struct stopmachine_data {
> > >         unsigned int fn_cpu;
> > >       int fn_result;
> > > -    int (*fn)(void *);
> > > +    int (*fn)(void *data);
> > >       void *fn_data;
> > >   };
> > 
> > At least one of the possible function used here calls the parameter
> > "arg", see take_cpu_down. But I don't think it is a MISRA requirement to
> > also harmonize those?
> > 
> > 
> > > @@ -73,7 +73,7 @@ static void stopmachine_wait_state(void)
> > >    * mandatory to be called only on an idle vcpu, as otherwise active core
> > >    * scheduling might hang.
> > >    */
> > > -int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu)
> > > +int stop_machine_run(int (*fn)(void *data), void *data, unsigned int cpu)
> > >   {
> > >       unsigned int i, nr_cpus;
> > >       unsigned int this = smp_processor_id();
> > > diff --git a/xen/common/tasklet.c b/xen/common/tasklet.c
> > > index 3ad67b5c24..3649798e6b 100644
> > > --- a/xen/common/tasklet.c
> > > +++ b/xen/common/tasklet.c
> > > @@ -199,7 +199,7 @@ static void migrate_tasklets_from_cpu(unsigned int
> > > cpu, struct list_head *list)
> > >       spin_unlock_irqrestore(&tasklet_lock, flags);
> > >   }
> > >   -void tasklet_init(struct tasklet *t, void (*func)(void *), void *data)
> > > +void tasklet_init(struct tasklet *t, void (*func)(void *data), void
> > > *data)
> > >   {
> > >       memset(t, 0, sizeof(*t));
> > >       INIT_LIST_HEAD(&t->list);
> > > @@ -208,7 +208,8 @@ void tasklet_init(struct tasklet *t, void (*func)(void
> > > *), void *data)
> > >       t->data = data;
> > >   }
> > >   -void softirq_tasklet_init(struct tasklet *t, void (*func)(void *), void
> > > *data)
> > > +void softirq_tasklet_init(struct tasklet *t,
> > > +                          void (*func)(void *data), void *data)
> > >   {
> > >       tasklet_init(t, func, data);
> > >       t->is_softirq = 1;
> > > diff --git a/xen/common/timer.c b/xen/common/timer.c
> > > index 0fddfa7487..bf7792dcb3 100644
> > > --- a/xen/common/timer.c
> > > +++ b/xen/common/timer.c
> > > @@ -291,7 +291,7 @@ static bool active_timer(const struct timer *timer)
> > >     void init_timer(
> > >       struct timer *timer,
> > > -    void        (*function)(void *),
> > > +    void        (*function)(void *data),
> > >       void         *data,
> > >       unsigned int  cpu)
> > >   {
> > > @@ -441,7 +441,7 @@ void kill_timer(struct timer *timer)
> > >     static void execute_timer(struct timers *ts, struct timer *t)
> > >   {
> > > -    void (*fn)(void *) = t->function;
> > > +    void (*fn)(void *data) = t->function;
> > >       void *data = t->data;
> > >         t->status = TIMER_STATUS_inactive;
> > > diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
> > > index 135f33f606..390f7b6082 100644
> > > --- a/xen/include/xen/rangeset.h
> > > +++ b/xen/include/xen/rangeset.h
> > > @@ -68,7 +68,7 @@ bool_t __must_check rangeset_overlaps_range(
> > >       struct rangeset *r, unsigned long s, unsigned long e);
> > >   int rangeset_report_ranges(
> > >       struct rangeset *r, unsigned long s, unsigned long e,
> > > -    int (*cb)(unsigned long s, unsigned long e, void *), void *ctxt);
> > > +    int (*cb)(unsigned long s, unsigned long e, void *data), void *ctxt);
> > 
> > Also here some of the functions use "arg" instead of ctxt
> > 
> > 
> > >   /*
> > >    * Note that the consume function can return an error value apart from
> > > @@ -77,7 +77,7 @@ int rangeset_report_ranges(
> > >    */
> > >   int rangeset_consume_ranges(struct rangeset *r,
> > >                               int (*cb)(unsigned long s, unsigned long e,
> > > -                                      void *, unsigned long *c),
> > > +                                      void *ctxt, unsigned long *c),
> > >                               void *ctxt);
> > 
> > Also here some of the functions use "dom" like irq_remove_cb.
> > 
> > 
> > But I actually like the patch as is, so if that's OK from a MISRA point
> > of view then I would give my reviewed-by.
> 
> Yes, this is OK for MISRA.


Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>