[XEN,5/6] x86/xstate: address MISRA C:2012 Rule 5.3

Commit Message

Nicola Vetrini Aug. 4, 2023, 3:27 p.m. UTC

Rename the local variables to avoid clashing with function 'xstate'
defined below, but declared in the corresponding header file.

No functional changes.

Signed-off-by: Nicola Vetrini <[e-mail]>
---
 xen/arch/x86/xstate.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Stefano Stabellini Aug. 4, 2023, 9:26 p.m. UTC | #1

On Fri, 4 Aug 2023, Nicola Vetrini wrote:
> Rename the local variables to avoid clashing with function 'xstate'
> defined below, but declared in the corresponding header file.
> 
> No functional changes.
> 
> Signed-off-by: Nicola Vetrini <[e-mail]>

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

Jan Beulich Aug. 7, 2023, 8:23 a.m. UTC | #2

On 04.08.2023 17:27, Nicola Vetrini wrote:
> Rename the local variables to avoid clashing with function 'xstate'
> defined below, but declared in the corresponding header file.

Hmm, there are two functions with such a local variable, but you don't
change those. You change "xsave" instead. The new name you use you took
from older functions afaict; newer ones use "xstate" (and use of this
name is extended in pending patches), so preferably we would follow
that naming model (and eventually rename all "xsave_area" as well).

Also - does "below" really matter and hence warrant the "but"?

Jan

Nicola Vetrini Aug. 7, 2023, 9:20 a.m. UTC | #3

On 07/08/2023 10:23, Jan Beulich wrote:
> On 04.08.2023 17:27, Nicola Vetrini wrote:
>> Rename the local variables to avoid clashing with function 'xstate'
>> defined below, but declared in the corresponding header file.
> 
> Hmm, there are two functions with such a local variable, but you don't
> change those. You change "xsave" instead. The new name you use you took
> from older functions afaict; newer ones use "xstate" (and use of this
> name is extended in pending patches), so preferably we would follow
> that naming model (and eventually rename all "xsave_area" as well).
> 
> Also - does "below" really matter and hence warrant the "but"?
> 
> Jan

I made a typo in the commit message. Indeed 'xsave' here is the culprit. 
I think 'xstate'
is ok for a rename, as it does not shadow anything  afaict.

Patch

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 3d566252ea..180455b26d 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -174,10 +174,10 @@  static void setup_xstate_comp(uint16_t *comp_offsets,
  */
 void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
 {
-    const struct xsave_struct *xsave = v->arch.xsave_area;
+    const struct xsave_struct *xsave_area = v->arch.xsave_area;
     const void *src;
     uint16_t comp_offsets[sizeof(xfeature_mask)*8];
-    u64 xstate_bv = xsave->xsave_hdr.xstate_bv;
+    u64 xstate_bv = xsave_area->xsave_hdr.xstate_bv;
     u64 valid;
 
     /* Check there is state to serialise (i.e. at least an XSAVE_HDR) */
@@ -185,19 +185,19 @@  void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
     /* Check there is the correct room to decompress into. */
     BUG_ON(size != xstate_ctxt_size(v->arch.xcr0_accum));
 
-    if ( !(xsave->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED) )
+    if ( !(xsave_area->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED) )
     {
-        memcpy(dest, xsave, size);
+        memcpy(dest, xsave_area, size);
         return;
     }
 
-    ASSERT(xsave_area_compressed(xsave));
-    setup_xstate_comp(comp_offsets, xsave->xsave_hdr.xcomp_bv);
+    ASSERT(xsave_area_compressed(xsave_area));
+    setup_xstate_comp(comp_offsets, xsave_area->xsave_hdr.xcomp_bv);
 
     /*
      * Copy legacy XSAVE area and XSAVE hdr area.
      */
-    memcpy(dest, xsave, XSTATE_AREA_MIN_SIZE);
+    memcpy(dest, xsave_area, XSTATE_AREA_MIN_SIZE);
     memset(dest + XSTATE_AREA_MIN_SIZE, 0, size - XSTATE_AREA_MIN_SIZE);
 
     ((struct xsave_struct *)dest)->xsave_hdr.xcomp_bv =  0;
@@ -206,7 +206,7 @@  void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
      * Copy each region from the possibly compacted offset to the
      * non-compacted offset.
      */
-    src = xsave;
+    src = xsave_area;
     valid = xstate_bv & ~XSTATE_FP_SSE;
     while ( valid )
     {
@@ -239,7 +239,7 @@  void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
  */
 void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
 {
-    struct xsave_struct *xsave = v->arch.xsave_area;
+    struct xsave_struct *xsave_area = v->arch.xsave_area;
     void *dest;
     uint16_t comp_offsets[sizeof(xfeature_mask)*8];
     u64 xstate_bv, valid;
@@ -252,7 +252,7 @@  void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
 
     if ( !(v->arch.xcr0_accum & XSTATE_XSAVES_ONLY) )
     {
-        memcpy(xsave, src, size);
+        memcpy(xsave_area, src, size);
         return;
     }
 
@@ -260,19 +260,19 @@  void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
      * Copy legacy XSAVE area, to avoid complications with CPUID
      * leaves 0 and 1 in the loop below.
      */
-    memcpy(xsave, src, FXSAVE_SIZE);
+    memcpy(xsave_area, src, FXSAVE_SIZE);
 
     /* Set XSTATE_BV and XCOMP_BV.  */
-    xsave->xsave_hdr.xstate_bv = xstate_bv;
-    xsave->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED;
+    xsave_area->xsave_hdr.xstate_bv = xstate_bv;
+    xsave_area->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED;
 
-    setup_xstate_comp(comp_offsets, xsave->xsave_hdr.xcomp_bv);
+    setup_xstate_comp(comp_offsets, xsave_area->xsave_hdr.xcomp_bv);
 
     /*
      * Copy each region from the non-compacted offset to the
      * possibly compacted offset.
      */
-    dest = xsave;
+    dest = xsave_area;
     valid = xstate_bv & ~XSTATE_FP_SSE;
     while ( valid )
     {