Re: [Xen-devel] [PATCH v6 2/4] xen/arm: use SYMBOL when required

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.01.19 at 17:58, <sstabellini@xxxxxxxxxx> wrote:
> On Fri, 11 Jan 2019, Jan Beulich wrote:
>> >> > --- a/xen/arch/arm/setup.c
>> >> > +++ b/xen/arch/arm/setup.c
>> >> > @@ -772,8 +772,10 @@ void __init start_xen(unsigned long 
>> >> > boot_phys_offset,
>> >> >  
>> >> >      /* Register Xen's load address as a boot module. */
>> >> >      xen_bootmodule = add_boot_module(BOOTMOD_XEN,
>> >> > -                             (paddr_t)(uintptr_t)(_start + 
>> >> > boot_phys_offset),
>> >> > -                             (paddr_t)(uintptr_t)(_end - _start + 1), 
>> >> > false);
>> >> > +                             (paddr_t)(uintptr_t)(SYMBOL(_start) +
>> >> > +                                                  boot_phys_offset),
>> >> > +                             (paddr_t)(uintptr_t)(SYMBOL(_end) -
>> >> > +                                                  SYMBOL(_start) + 1), 
>> >> > false);
>> >> 
>> >> Why you need the double casts, i.e. why does (uintptr_t) alone not
>> >> suffice?
>> > 
>> > The original reason was just not to change the existing code outside of
>> > adding SYMBOL :-)
>> > 
>> > But to answer your question, uintptr_t is the same size of char*, while
>> > paddr_t is always 64bit. uintptr_t casts to integer type, paddr_t casts
>> > to the right size. I don't think it is allowed to change from pointer to
>> > integer and change integer size in a single cast.
>> 
>> Correct, but that's not what I've been asking for. Instead I'd like
>> to see the (paddr_t) casts dropped, at least if this was in code I'm
>> the maintainer for.
> 
> But add_boot_module takes paddr_t as arguments. Why would you want the
> explicit cast dropped?

Yes. There are very, very many places where we assume the compiler
to do the right thing when changing width of integer types. I simply
see no reason why we would want to diverge here. If the compiler
warned about truncating conversions (like some others do), then
there would be a reason to maintain explicit down-casts, but here it's
an up-cast in all cases afaict, and iirc there's no undefined behavior,
implementation defined behavior, or alike associated with widening of
integer types.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel