Re: [PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion
Il 25/07/2014 19:18, Rustad, Mark D ha scritto:
On Jul 25, 2014, at 7:06 AM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
From: Mark Rustad mark.d.rus...@intel.com
Resolve shadow warnings that appear in W=2 builds. In this case,
a macro declared an inner local variable with the same name as an
outer one. This can be a serious hazard in the event that the outer
variable is ever passed as a parameter, as the inner variable will
be referenced instead of what was intended. This macro doesn't have
any parameters - at this time - but prepend an _ to all of the
macro-declared variables as is the custom, to resolve the warnings
and eliminate any future hazard.
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
Signed-off-by: Jeff Kirsher jeffrey.t.kirs...@intel.com
---
arch/x86/kvm/mmutrace.h | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index 9d2e0ff..2d8d00c 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -22,26 +22,26 @@
__entry-unsync = sp-unsync;
#define KVM_MMU_PAGE_PRINTK() ({\
- const char *ret = p-buffer + p-len; \
- static const char *access_str[] = { \
+ const char *_ret = p-buffer + p-len; \
+ static const char *_access_str[] = {\
---, --x, w--, w-x, -u-, -ux, wu-, wux \
}; \
- union kvm_mmu_page_role role; \
+ union kvm_mmu_page_role _role; \
\
- role.word = __entry-role; \
+ _role.word = __entry-role; \
\
trace_seq_printf(p, sp gen %lx gfn %llx %u%s q%u%s %s%s \
%snxe root %u %s%c, __entry-mmu_valid_gen, \
-__entry-gfn, role.level, \
-role.cr4_pae ? pae : ,\
-role.quadrant, \
-role.direct ? direct : , \
-access_str[role.access], \
-role.invalid ? invalid : ,\
-role.nxe ? : !, \
+__entry-gfn, _role.level, \
+_role.cr4_pae ? pae : , \
+_role.quadrant,\
+_role.direct ? direct : , \
+_access_str[_role.access], \
+_role.invalid ? invalid : , \
+_role.nxe ? : !, \
__entry-root_count, \
__entry-unsync ? unsync : sync, 0); \
- ret;\
+ _ret; \
})
#define kvm_mmu_trace_pferr_flags \
I think this unnecessarily uglifies the code, so I am not applying it.
Gleb, what do you think?
Would you consider a version that only changes ret to _ret?
That would be more acceptable, or even better I guess you could stash
away p-len and return p-buffer + saved_len.
The thing is that it makes no sense for the caller's ret to alias any
of the other local variables of the macro. So the code gets uglier but
the only one to benefit is the compiler.
Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion
From: Mark Rustad mark.d.rus...@intel.com
Resolve shadow warnings that appear in W=2 builds. In this case,
a macro declared an inner local variable with the same name as an
outer one. This can be a serious hazard in the event that the outer
variable is ever passed as a parameter, as the inner variable will
be referenced instead of what was intended. This macro doesn't have
any parameters - at this time - but prepend an _ to all of the
macro-declared variables as is the custom, to resolve the warnings
and eliminate any future hazard.
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
Signed-off-by: Jeff Kirsher jeffrey.t.kirs...@intel.com
---
arch/x86/kvm/mmutrace.h | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index 9d2e0ff..2d8d00c 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -22,26 +22,26 @@
__entry-unsync = sp-unsync;
#define KVM_MMU_PAGE_PRINTK() ({ \
- const char *ret = p-buffer + p-len; \
- static const char *access_str[] = { \
+ const char *_ret = p-buffer + p-len; \
+ static const char *_access_str[] = {\
---, --x, w--, w-x, -u-, -ux, wu-, wux \
}; \
- union kvm_mmu_page_role role; \
+ union kvm_mmu_page_role _role; \
\
- role.word = __entry-role; \
+ _role.word = __entry-role; \
\
trace_seq_printf(p, sp gen %lx gfn %llx %u%s q%u%s %s%s \
%snxe root %u %s%c, __entry-mmu_valid_gen, \
-__entry-gfn, role.level, \
-role.cr4_pae ? pae : ,\
-role.quadrant, \
-role.direct ? direct : , \
-access_str[role.access], \
-role.invalid ? invalid : ,\
-role.nxe ? : !, \
+__entry-gfn, _role.level, \
+_role.cr4_pae ? pae : , \
+_role.quadrant,\
+_role.direct ? direct : , \
+_access_str[_role.access], \
+_role.invalid ? invalid : , \
+_role.nxe ? : !, \
__entry-root_count, \
__entry-unsync ? unsync : sync, 0); \
- ret;\
+ _ret; \
})
#define kvm_mmu_trace_pferr_flags \
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion
Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
From: Mark Rustad mark.d.rus...@intel.com
Resolve shadow warnings that appear in W=2 builds. In this case,
a macro declared an inner local variable with the same name as an
outer one. This can be a serious hazard in the event that the outer
variable is ever passed as a parameter, as the inner variable will
be referenced instead of what was intended. This macro doesn't have
any parameters - at this time - but prepend an _ to all of the
macro-declared variables as is the custom, to resolve the warnings
and eliminate any future hazard.
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
Signed-off-by: Jeff Kirsher jeffrey.t.kirs...@intel.com
---
arch/x86/kvm/mmutrace.h | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index 9d2e0ff..2d8d00c 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -22,26 +22,26 @@
__entry-unsync = sp-unsync;
#define KVM_MMU_PAGE_PRINTK() ({ \
- const char *ret = p-buffer + p-len; \
- static const char *access_str[] = { \
+ const char *_ret = p-buffer + p-len; \
+ static const char *_access_str[] = {\
---, --x, w--, w-x, -u-, -ux, wu-, wux \
}; \
- union kvm_mmu_page_role role; \
+ union kvm_mmu_page_role _role; \
\
- role.word = __entry-role; \
+ _role.word = __entry-role; \
\
trace_seq_printf(p, sp gen %lx gfn %llx %u%s q%u%s %s%s \
%snxe root %u %s%c, __entry-mmu_valid_gen, \
- __entry-gfn, role.level, \
- role.cr4_pae ? pae : ,\
- role.quadrant, \
- role.direct ? direct : , \
- access_str[role.access], \
- role.invalid ? invalid : ,\
- role.nxe ? : !, \
+ __entry-gfn, _role.level, \
+ _role.cr4_pae ? pae : , \
+ _role.quadrant,\
+ _role.direct ? direct : , \
+ _access_str[_role.access], \
+ _role.invalid ? invalid : , \
+ _role.nxe ? : !, \
__entry-root_count, \
__entry-unsync ? unsync : sync, 0); \
- ret;\
+ _ret; \
})
#define kvm_mmu_trace_pferr_flags \
I think this unnecessarily uglifies the code, so I am not applying it.
Gleb, what do you think?
Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion
On Jul 25, 2014, at 7:06 AM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
From: Mark Rustad mark.d.rus...@intel.com
Resolve shadow warnings that appear in W=2 builds. In this case,
a macro declared an inner local variable with the same name as an
outer one. This can be a serious hazard in the event that the outer
variable is ever passed as a parameter, as the inner variable will
be referenced instead of what was intended. This macro doesn't have
any parameters - at this time - but prepend an _ to all of the
macro-declared variables as is the custom, to resolve the warnings
and eliminate any future hazard.
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
Signed-off-by: Jeff Kirsher jeffrey.t.kirs...@intel.com
---
arch/x86/kvm/mmutrace.h | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index 9d2e0ff..2d8d00c 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -22,26 +22,26 @@
__entry-unsync = sp-unsync;
#define KVM_MMU_PAGE_PRINTK() ({ \
-const char *ret = p-buffer + p-len; \
-static const char *access_str[] = { \
+const char *_ret = p-buffer + p-len; \
+static const char *_access_str[] = {\
---, --x, w--, w-x, -u-, -ux, wu-, wux \
}; \
-union kvm_mmu_page_role role; \
+union kvm_mmu_page_role _role; \
\
-role.word = __entry-role; \
+_role.word = __entry-role; \
\
trace_seq_printf(p, sp gen %lx gfn %llx %u%s q%u%s %s%s \
%snxe root %u %s%c, __entry-mmu_valid_gen, \
- __entry-gfn, role.level, \
- role.cr4_pae ? pae : ,\
- role.quadrant, \
- role.direct ? direct : , \
- access_str[role.access], \
- role.invalid ? invalid : ,\
- role.nxe ? : !, \
+ __entry-gfn, _role.level, \
+ _role.cr4_pae ? pae : , \
+ _role.quadrant,\
+ _role.direct ? direct : , \
+ _access_str[_role.access], \
+ _role.invalid ? invalid : , \
+ _role.nxe ? : !, \
__entry-root_count, \
__entry-unsync ? unsync : sync, 0); \
-ret;\
+_ret; \
})
#define kvm_mmu_trace_pferr_flags \
I think this unnecessarily uglifies the code, so I am not applying it.
Gleb, what do you think?
Would you consider a version that only changes ret to _ret? That would be
enough to resolve the warning. I only did the other variables because it seemed
to be a best practice for these inner block declarations.
Hmmm. It seems like p should really be a parameter to this macro.
--
Mark Rustad, Networking Division, Intel Corporation
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
