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.vetr...@bugseng.com>
---
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)