| Message ID | 591D7A6C020000780015AC05@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 18/05/17 09:41, Jan Beulich wrote: > The compiler dislikes duplicat "const", and the ones it complains about s/duplicat/duplicate/ > look like they we in fact meant to be placed differently. > > Also fix array_access_okay() (just like on x86), despite the construct > being unused on ARM: -Wint-in-bool-context, enabled by default in > gcc 7, doesn't like multiplication in conditional operators. "Hide" it, > at the risk of the next compiler version becoming smarter and > recognizing even that. (The hope is that added smartness then would > also better deal with legitimate cases like the one here.) The change > could have been done in access_ok(), but I think we better keep it at > the place the compiler is actually unhappy about. I am wondering if we should drop array_access_ok and access_ok as they are not used. Anyway, I am happy with both way: Reviewed-by: Julien Grall <julien.grall@arm.com> Cheers,
On Thu, 18 May 2017, Julien Grall wrote: > Hi, > > On 18/05/17 09:41, Jan Beulich wrote: > > The compiler dislikes duplicat "const", and the ones it complains about > > s/duplicat/duplicate/ > > > look like they we in fact meant to be placed differently. > > > > Also fix array_access_okay() (just like on x86), despite the construct > > being unused on ARM: -Wint-in-bool-context, enabled by default in > > gcc 7, doesn't like multiplication in conditional operators. "Hide" it, > > at the risk of the next compiler version becoming smarter and > > recognizing even that. (The hope is that added smartness then would > > also better deal with legitimate cases like the one here.) The change > > could have been done in access_ok(), but I think we better keep it at > > the place the compiler is actually unhappy about. > > I am wondering if we should drop array_access_ok and access_ok as they are not > used. > > Anyway, I am happy with both way: > > Reviewed-by: Julien Grall <julien.grall@arm.com> Is this series for 4.9?
>>> On 18.05.17 at 17:12, <julien.grall@arm.com> wrote: > On 18/05/17 09:41, Jan Beulich wrote: >> The compiler dislikes duplicat "const", and the ones it complains about > > s/duplicat/duplicate/ > >> look like they we in fact meant to be placed differently. >> >> Also fix array_access_okay() (just like on x86), despite the construct >> being unused on ARM: -Wint-in-bool-context, enabled by default in >> gcc 7, doesn't like multiplication in conditional operators. "Hide" it, >> at the risk of the next compiler version becoming smarter and >> recognizing even that. (The hope is that added smartness then would >> also better deal with legitimate cases like the one here.) The change >> could have been done in access_ok(), but I think we better keep it at >> the place the compiler is actually unhappy about. > > I am wondering if we should drop array_access_ok and access_ok as they > are not used. I did consider this too, but thought that such a decision (and patch) would better come from someone closer to ARM. > Anyway, I am happy with both way: > > Reviewed-by: Julien Grall <julien.grall@arm.com> Thanks, Jan
>>> On 18.05.17 at 20:35, <sstabellini@kernel.org> wrote: > On Thu, 18 May 2017, Julien Grall wrote: >> Hi, >> >> On 18/05/17 09:41, Jan Beulich wrote: >> > The compiler dislikes duplicat "const", and the ones it complains about >> >> s/duplicat/duplicate/ >> >> > look like they we in fact meant to be placed differently. >> > >> > Also fix array_access_okay() (just like on x86), despite the construct >> > being unused on ARM: -Wint-in-bool-context, enabled by default in >> > gcc 7, doesn't like multiplication in conditional operators. "Hide" it, >> > at the risk of the next compiler version becoming smarter and >> > recognizing even that. (The hope is that added smartness then would >> > also better deal with legitimate cases like the one here.) The change >> > could have been done in access_ok(), but I think we better keep it at >> > the place the compiler is actually unhappy about. >> >> I am wondering if we should drop array_access_ok and access_ok as they are not >> used. >> >> Anyway, I am happy with both way: >> >> Reviewed-by: Julien Grall <julien.grall@arm.com> > > Is this series for 4.9? Yes, as mentioned in 0/3. Jan
Hi Jan, On 19/05/2017 07:16, Jan Beulich wrote: >>>> On 18.05.17 at 17:12, <julien.grall@arm.com> wrote: >> On 18/05/17 09:41, Jan Beulich wrote: >>> The compiler dislikes duplicat "const", and the ones it complains about >> >> s/duplicat/duplicate/ >> >>> look like they we in fact meant to be placed differently. >>> >>> Also fix array_access_okay() (just like on x86), despite the construct >>> being unused on ARM: -Wint-in-bool-context, enabled by default in >>> gcc 7, doesn't like multiplication in conditional operators. "Hide" it, >>> at the risk of the next compiler version becoming smarter and >>> recognizing even that. (The hope is that added smartness then would >>> also better deal with legitimate cases like the one here.) The change >>> could have been done in access_ok(), but I think we better keep it at >>> the place the compiler is actually unhappy about. >> >> I am wondering if we should drop array_access_ok and access_ok as they >> are not used. > > I did consider this too, but thought that such a decision (and patch) > would better come from someone closer to ARM. I will send a patch for this patch after 4.9. Cheers, > >> Anyway, I am happy with both way: >> >> Reviewed-by: Julien Grall <julien.grall@arm.com> > > Thanks, Jan >
--- a/xen/arch/arm/platforms/brcm.c +++ b/xen/arch/arm/platforms/brcm.c @@ -271,7 +271,7 @@ static __init int brcm_init(void) return brcm_populate_plat_regs(); } -static const char const *brcm_dt_compat[] __initconst = +static const char *const brcm_dt_compat[] __initconst = { "brcm,bcm7445d0", NULL --- a/xen/arch/arm/platforms/rcar2.c +++ b/xen/arch/arm/platforms/rcar2.c @@ -46,7 +46,7 @@ static int __init rcar2_smp_init(void) return 0; } -static const char const *rcar2_dt_compat[] __initdata = +static const char *const rcar2_dt_compat[] __initconst = { "renesas,lager", NULL --- a/xen/include/asm-arm/guest_access.h +++ b/xen/include/asm-arm/guest_access.h @@ -8,7 +8,8 @@ #define access_ok(addr,size) (1) #define array_access_ok(addr,count,size) \ - (likely(count < (~0UL/size)) && access_ok(addr,count*size)) + (likely((count) < (~0UL / (size))) && \ + access_ok(addr, 0 + (count) * (size))) unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len); unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,