[XEN,v2] x86/mctelem: address violations of MISRA C: 2012 Rule 5.3

Commit Message

Nicola Vetrini June 26, 2024, 10:48 a.m. UTC

From: Alessandro Zucchelli <[e-mail]>

This addresses violations of MISRA C:2012 Rule 5.3 which states as
following: An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope.

In this case the gloabl variable being shadowed is the global static struct
mctctl in this file, therefore the local variables are renamed to avoid this.

No functional change.

Signed-off-by: Alessandro Zucchelli <[e-mail]>
Signed-off-by: Nicola Vetrini <[e-mail]>
---
Changes in v2:
- s/mctctl_cpu/ctl/ and amended file comment and commit message
---
 xen/arch/x86/cpu/mcheck/mctelem.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

Comments

Stefano Stabellini June 27, 2024, 12:57 a.m. UTC | #1

On Wed, 26 Jun 2024, Nicola Vetrini wrote:
> From: Alessandro Zucchelli <[e-mail]>
> 
> This addresses violations of MISRA C:2012 Rule 5.3 which states as
> following: An identifier declared in an inner scope shall not hide an
> identifier declared in an outer scope.
> 
> In this case the gloabl variable being shadowed is the global static struct
> mctctl in this file, therefore the local variables are renamed to avoid this.
> 
> No functional change.
> 
> Signed-off-by: Alessandro Zucchelli <[e-mail]>
> Signed-off-by: Nicola Vetrini <[e-mail]>

Nice one!

Reviewed-by: Stefano Stabellini <[e-mail]>

Jan Beulich July 1, 2024, 8:27 a.m. UTC | #2

On 27.06.2024 02:57, Stefano Stabellini wrote:
> On Wed, 26 Jun 2024, Nicola Vetrini wrote:
>> From: Alessandro Zucchelli <[e-mail]>
>>
>> This addresses violations of MISRA C:2012 Rule 5.3 which states as
>> following: An identifier declared in an inner scope shall not hide an
>> identifier declared in an outer scope.
>>
>> In this case the gloabl variable being shadowed is the global static struct
>> mctctl in this file, therefore the local variables are renamed to avoid this.

"global" and "static" contradict one another; I think you mean "file scope".
While there (nit) also s/gloabl/global/ or perhaps even s/gloabl// to deal
with the conflict with "static" also there (without writing "file scope"
twice).

>> No functional change.
>>
>> Signed-off-by: Alessandro Zucchelli <[e-mail]>
>> Signed-off-by: Nicola Vetrini <[e-mail]>
> 
> Nice one!
> 
> Reviewed-by: Stefano Stabellini <[e-mail]>

With the adjustment:
Acked-by: Jan Beulich <[e-mail]>

Jan

Nicola Vetrini July 1, 2024, 9:19 a.m. UTC | #3

On 2024-07-01 10:27, Jan Beulich wrote:
> On 27.06.2024 02:57, Stefano Stabellini wrote:
>> On Wed, 26 Jun 2024, Nicola Vetrini wrote:
>>> From: Alessandro Zucchelli <[e-mail]>
>>> 
>>> This addresses violations of MISRA C:2012 Rule 5.3 which states as
>>> following: An identifier declared in an inner scope shall not hide an
>>> identifier declared in an outer scope.
>>> 
>>> In this case the gloabl variable being shadowed is the global static 
>>> struct
>>> mctctl in this file, therefore the local variables are renamed to 
>>> avoid this.
> 
> "global" and "static" contradict one another; I think you mean "file 
> scope".

Yes.

> While there (nit) also s/gloabl/global/ or perhaps even s/gloabl// to 
> deal
> with the conflict with "static" also there (without writing "file 
> scope"
> twice).
> 
>>> No functional change.
>>> 
>>> Signed-off-by: Alessandro Zucchelli 
>>> <[e-mail]>
>>> Signed-off-by: Nicola Vetrini <[e-mail]>
>> 
>> Nice one!
>> 
>> Reviewed-by: Stefano Stabellini <[e-mail]>
> 
> With the adjustment:
> Acked-by: Jan Beulich <[e-mail]>
> 
> Jan

Patch

diff --git a/xen/arch/x86/cpu/mcheck/mctelem.c b/xen/arch/x86/cpu/mcheck/mctelem.c
index b8d0368a7d37..123e4102adca 100644
--- a/xen/arch/x86/cpu/mcheck/mctelem.c
+++ b/xen/arch/x86/cpu/mcheck/mctelem.c
@@ -168,28 +168,28 @@  static void mctelem_xchg_head(struct mctelem_ent **headp,
 void mctelem_defer(mctelem_cookie_t cookie, bool lmce)
 {
 	struct mctelem_ent *tep = COOKIE2MCTE(cookie);
-	struct mc_telem_cpu_ctl *mctctl = &this_cpu(mctctl);
+	struct mc_telem_cpu_ctl *ctl = &this_cpu(mctctl);
 
-	ASSERT(mctctl->pending == NULL || mctctl->lmce_pending == NULL);
+	ASSERT(ctl->pending == NULL || ctl->lmce_pending == NULL);
 
-	if (mctctl->pending)
-		mctelem_xchg_head(&mctctl->pending, &tep->mcte_next, tep);
+	if (ctl->pending)
+		mctelem_xchg_head(&ctl->pending, &tep->mcte_next, tep);
 	else if (lmce)
-		mctelem_xchg_head(&mctctl->lmce_pending, &tep->mcte_next, tep);
+		mctelem_xchg_head(&ctl->lmce_pending, &tep->mcte_next, tep);
 	else {
 		/*
 		 * LMCE is supported on Skylake-server and later CPUs, on
 		 * which mce_broadcast is always true. Therefore, non-empty
-		 * mctctl->lmce_pending in this branch implies a broadcasting
+		 * ctl->lmce_pending in this branch implies a broadcasting
 		 * MC# is being handled, every CPU is in the exception
-		 * context, and no one is consuming mctctl->pending at this
+		 * context, and no one is consuming ctl->pending at this
 		 * moment. As a result, the following two exchanges together
 		 * can be treated as atomic.
 		 */
-		if (mctctl->lmce_pending)
-			mctelem_xchg_head(&mctctl->lmce_pending,
-					  &mctctl->pending, NULL);
-		mctelem_xchg_head(&mctctl->pending, &tep->mcte_next, tep);
+		if (ctl->lmce_pending)
+			mctelem_xchg_head(&ctl->lmce_pending,
+					  &ctl->pending, NULL);
+		mctelem_xchg_head(&ctl->pending, &tep->mcte_next, tep);
 	}
 }
 
@@ -213,7 +213,7 @@  void mctelem_process_deferred(unsigned int cpu,
 {
 	struct mctelem_ent *tep;
 	struct mctelem_ent *head, *prev;
-	struct mc_telem_cpu_ctl *mctctl = &per_cpu(mctctl, cpu);
+	struct mc_telem_cpu_ctl *ctl = &per_cpu(mctctl, cpu);
 	int ret;
 
 	/*
@@ -232,7 +232,7 @@  void mctelem_process_deferred(unsigned int cpu,
 	 * Any MC# occurring after the following atomic exchange will be
 	 * handled by another round of MCE softirq.
 	 */
-	mctelem_xchg_head(lmce ? &mctctl->lmce_pending : &mctctl->pending,
+	mctelem_xchg_head(lmce ? &ctl->lmce_pending : &ctl->pending,
 			  &this_cpu(mctctl.processing), NULL);
 
 	head = this_cpu(mctctl.processing);