[PATCH v2] x86/PV: rename a local variable in pv_emulate_gate_op()

Jan Beulich posted 1 patch 5 days, 13 hours ago
Failed in applying to current master (apply log)
[PATCH v2] x86/PV: rename a local variable in pv_emulate_gate_op()
Posted by Jan Beulich 5 days, 13 hours ago
... shadowing a function scope one, thus violating Misra C:2012 rule 5.3
("An identifier declared in an inner scope shall not hide an identifier
declared in an outer scope"). No difference in generated code.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Technically, as the outer scope "rc" isn't used again later, we could
simply drop the inner decl. That seemed more error prone to me, though.
---
v2: Re-base.

--- a/xen/arch/x86/pv/emul-gate-op.c
+++ b/xen/arch/x86/pv/emul-gate-op.c
@@ -287,17 +287,17 @@ void pv_emulate_gate_op(struct cpu_user_
     {
         unsigned int ss, esp, *stkp;
         uint32_t value;
-        int rc;
+        int left;
 #define push(item) do \
         { \
             value = (item); \
             --stkp; \
             esp -= 4; \
-            rc = __copy_to_guest_pv(stkp, &value, sizeof(value)); \
-            if ( rc ) \
+            left = __copy_to_guest_pv(stkp, &value, sizeof(value)); \
+            if ( left ) \
             { \
                 pv_inject_page_fault(PFEC_write_access, \
-                                     (unsigned long)(stkp + 1) - rc); \
+                                     (unsigned long)(stkp + 1) - left); \
                 return; \
             } \
         } while ( 0 )
@@ -361,10 +361,11 @@ void pv_emulate_gate_op(struct cpu_user_
                     unsigned int parm;
 
                     --ustkp;
-                    rc = __copy_from_guest_pv(&parm, ustkp, sizeof(parm));
-                    if ( rc )
+                    left = __copy_from_guest_pv(&parm, ustkp, sizeof(parm));
+                    if ( left )
                     {
-                        pv_inject_page_fault(0, (unsigned long)(ustkp + 1) - rc);
+                        pv_inject_page_fault(0,
+                                             (unsigned long)(ustkp + 1) - left);
                         return;
                     }
                     push(parm);
Re: [PATCH v2] x86/PV: rename a local variable in pv_emulate_gate_op()
Posted by Andrew Cooper 5 days, 13 hours ago
On 02/06/2026 2:20 pm, Jan Beulich wrote:
> ... shadowing a function scope one, thus violating Misra C:2012 rule 5.3
> ("An identifier declared in an inner scope shall not hide an identifier
> declared in an outer scope"). No difference in generated code.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Technically, as the outer scope "rc" isn't used again later, we could
> simply drop the inner decl. That seemed more error prone to me, though.

But it's consistent with how we use this pattern and naming for
injecting pagefaults elsewhere.

left (as in remaining) is a complicated name to use, because it's
ambiguous with left (vs right), but this is not a context where the
meaning is clear (e.g. the sort functions).

I think deleting the inner rc is the better way to go here.

~Andrew
Re: [PATCH v2] x86/PV: rename a local variable in pv_emulate_gate_op()
Posted by Jan Beulich 5 days, 13 hours ago
On 02.06.2026 15:29, Andrew Cooper wrote:
> On 02/06/2026 2:20 pm, Jan Beulich wrote:
>> ... shadowing a function scope one, thus violating Misra C:2012 rule 5.3
>> ("An identifier declared in an inner scope shall not hide an identifier
>> declared in an outer scope"). No difference in generated code.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Technically, as the outer scope "rc" isn't used again later, we could
>> simply drop the inner decl. That seemed more error prone to me, though.
> 
> But it's consistent with how we use this pattern and naming for
> injecting pagefaults elsewhere.
> 
> left (as in remaining) is a complicated name to use, because it's
> ambiguous with left (vs right), but this is not a context where the
> meaning is clear (e.g. the sort functions).
> 
> I think deleting the inner rc is the better way to go here.

Well, okay, can do, but: Couldn't you have said so on v1 already, so I
wouldn't have needed to make a v3 right after sending v2? Iirc you
pointed out the conflict with Teddy's fix, so you must have looked at
v1 ...

Jan
Re: [PATCH v2] x86/PV: rename a local variable in pv_emulate_gate_op()
Posted by Andrew Cooper 5 days, 13 hours ago
On 02/06/2026 2:39 pm, Jan Beulich wrote:
> On 02.06.2026 15:29, Andrew Cooper wrote:
>> On 02/06/2026 2:20 pm, Jan Beulich wrote:
>>> ... shadowing a function scope one, thus violating Misra C:2012 rule 5.3
>>> ("An identifier declared in an inner scope shall not hide an identifier
>>> declared in an outer scope"). No difference in generated code.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> Technically, as the outer scope "rc" isn't used again later, we could
>>> simply drop the inner decl. That seemed more error prone to me, though.
>> But it's consistent with how we use this pattern and naming for
>> injecting pagefaults elsewhere.
>>
>> left (as in remaining) is a complicated name to use, because it's
>> ambiguous with left (vs right), but this is not a context where the
>> meaning is clear (e.g. the sort functions).
>>
>> I think deleting the inner rc is the better way to go here.
> Well, okay, can do, but: Couldn't you have said so on v1 already, so I
> wouldn't have needed to make a v3 right after sending v2? Iirc you
> pointed out the conflict with Teddy's fix, so you must have looked at
> v1 ...

I'm sorry - I thought I had already fed this back (hence was surprised
at v2 looking like this), but I couldn't find any email.

~Andrew