Re: [XEN PATCH] xen/sched: mechanical renaming to address MISRA C:2012 Rule 5.3

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 24/07/23 10:26, Jan Beulich wrote:
> On 21.07.2023 17:31, Nicola Vetrini wrote:
> > Rule 5.3 has the following headline:
> > "An identifier declared in an inner scope shall not hide an
> > identifier declared in an outer scope"
> >
> > The renaming s/sched_id/scheduler_id of the function defined in
> > 'xen/common/sched/core.c' prevents any hiding of that function
> > by the many instances of omonymous function parameters.
> >
> > Similarly, the renames
> > - s/ops/operations
> > - s/do_softirq/exec_softirq
> > - s/loop/it
> > are introduced for parameter names, to avoid any conflict
> > with the homonymous variable or function defined in an enclosing
> > scope.
> >
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > ---
> >   xen/common/sched/core.c    | 18 +++++++++---------
> >   xen/common/sched/credit2.c |  4 ++--
> >   xen/common/sysctl.c        |  2 +-
> >   xen/include/xen/sched.h    |  2 +-
> >   4 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> > index 022f548652..e74b1208bd 100644
> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -99,13 +99,13 @@ static void sched_set_affinity(
> >       struct sched_unit *unit, const cpumask_t *hard, const cpumask_t *soft);
> >   
> >   static struct sched_resource *cf_check
> > -sched_idle_res_pick(const struct scheduler *ops, const struct sched_unit *unit)
> > +sched_idle_res_pick(const struct scheduler *operations, const struct 
> > sched_unit *unit)
> >   {
> >       return unit->res;
> >   }
> >   
> >   static void *cf_check
> > -sched_idle_alloc_udata(const struct scheduler *ops, struct sched_unit *unit,
> > +sched_idle_alloc_udata(const struct scheduler *operations, struct sched_unit 
> > *unit,
> >                          void *dd)
> >   {
> >       /* Any non-NULL pointer is fine here. */
> > @@ -113,12 +113,12 @@ sched_idle_alloc_udata(const struct scheduler *ops, 
> > struct sched_unit *unit,
> >   }
> >   
> >   static void cf_check
> > -sched_idle_free_udata(const struct scheduler *ops, void *priv)
> > +sched_idle_free_udata(const struct scheduler *operations, void *priv)
> >   {
> >   }
> >   
> >   static void cf_check sched_idle_schedule(
> > -    const struct scheduler *ops, struct sched_unit *unit, s_time_t now,
> > +    const struct scheduler *operations, struct sched_unit *unit, s_time_t now,
> >       bool tasklet_work_scheduled)
> >   {
> >       const unsigned int cpu = smp_processor_id();
> These renames bring the idle scheduler out of sync with all others. I
> think it wants considering to rename the file scope variable instead.
That's ok, since the variable is static.

> > @@ -2579,7 +2579,7 @@ static void cf_check sched_slave(void)
> >       struct sched_unit    *prev = vprev->sched_unit, *next;
> >       s_time_t              now;
> >       spinlock_t           *lock;
> > -    bool                  do_softirq = false;
> > +    bool                  exec_softirq = false;
> As an alternative to Stefano's suggestion, maybe consider "need_softirq"?
I like it, especially since it's a local variable.

> > --- a/xen/common/sched/credit2.c
> > +++ b/xen/common/sched/credit2.c
> > @@ -3884,7 +3884,7 @@ csched2_dump(const struct scheduler *ops)
> >       list_for_each_entry ( rqd, &prv->rql, rql )
> >       {
> >           struct list_head *iter, *runq = &rqd->runq;
> > -        int loop = 0;
> > +        int it = 0;
> Instead of renaming, why can't we just drop this second variable, re-using
> the outer scope one here (and at the same time doing away with a not really
> appropriate use of plain "int")? (This may then want accompanying by ...
>
> > @@ -3901,7 +3901,7 @@ csched2_dump(const struct scheduler *ops)
> >   
> >               if ( svc )
> >               {
> > -                printk("\t%3d: ", loop++);
> > +                printk("\t%3d: ", it++);
> ... switching to %3u here.)
Good point.

I'll submit a v2 shortly with these changes.

Regards,

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)