[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/8] gnttab: Remove unused-but-set variable
Hi Andrew, Jan On 27.04.2022 14:33, Andrew Cooper wrote: > On 27/04/2022 12:06, Michal Orzel wrote: >> Hi Jan, >> >> On 27.04.2022 11:59, Jan Beulich wrote: >>> On 27.04.2022 11:49, Michal Orzel wrote: >>>> Function unmap_common_complete defines and sets a variable ld that is >>>> later on passed to a macro gnttab_host_mapping_get_page_type. On arm >>>> this macro does not make use of any arguments causing a compiler to >>>> warn about unused-but-set variable (when -Wunused-but-set-variable is >>>> enabled). Fix this by removing ld and directly passing current->domain >>>> to gnttab_host_mapping_get_page_type. >>> I think we want to retain the ld / rd notation. Therefore I think it's >>> rather the Arm macro which wants adjusting to not leave this argument >>> unused. >>> >> I would agree provided that the ld variable was used in more than one place. >> As it is not, it does not seem very beneficial to keep a variable that is >> used >> just in one place and stores the macro value. >> >> When it comes to gnttab_host_mapping_get_page_type macro, on Arm it is >> defined as (0) >> so modifying it seems to be a quite big overhead. > > diff --git a/xen/arch/arm/include/asm/grant_table.h > b/xen/arch/arm/include/asm/grant_table.h > index d31a4d6805d6..9f68c2a37eb6 100644 > --- a/xen/arch/arm/include/asm/grant_table.h > +++ b/xen/arch/arm/include/asm/grant_table.h > @@ -31,10 +31,10 @@ static inline void gnttab_mark_dirty(struct domain > *d, mfn_t mfn) > > int create_grant_host_mapping(unsigned long gpaddr, mfn_t mfn, > unsigned int flags, unsigned int > cache_flags); > -#define gnttab_host_mapping_get_page_type(ro, ld, rd) (0) > +#define gnttab_host_mapping_get_page_type(ro, ld, rd) (ro, ld, rd, 0) > int replace_grant_host_mapping(unsigned long gpaddr, mfn_t mfn, > unsigned long new_gpaddr, unsigned int > flags); > -#define gnttab_release_host_mappings(domain) 1 > +#define gnttab_release_host_mappings(domain) (domain, 1) > > /* > * The region used by Xen on the memory will never be mapped in DOM0 > > It's about parameter evaluation, not about adding extra code when compiled. > Unfortunately, solution presented by Andrew does not work. We will get the following error due to -Werror=unused-value: "left-hand operand of comma expression has no effect" If we want to keep this variable, how about using unused attribute? struct domain *__maybe_unused ld Cheers, Michal
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |