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 <[email protected]>
> ---
> 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.
> @@ -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"?
> --- 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.)
Jan
