Re: [PATCH 3/4] x86/kvm: Resolve shadow warnings in macro expansion

Paolo Bonzini
2014-07-25T14:06:40Z

Il 25/07/2014 15:27, Jeff Kirsher ha scritto:
> From: Mark Rustad <mark.d.rustad@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.rustad@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@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