Re: [XEN PATCH 12/13] xen/x86: fixed violations of MISRA C:2012 Rule 7.2

[Jan Beulich <jbeulich@xxxxxxxx>]

First of all - please trim replies.

On 20.06.2023 23:23, Stefano Stabellini wrote:
> On Tue, 20 Jun 2023, Simone Ballarin wrote:
>> --- a/xen/arch/x86/percpu.c
>> +++ b/xen/arch/x86/percpu.c
>> @@ -12,7 +12,7 @@ unsigned long __per_cpu_offset[NR_CPUS];
>>   * possible #PF at (NULL + a little) which has security implications in the
>>   * context of PV guests.
>>   */
>> -#define INVALID_PERCPU_AREA (0x8000000000000000L - (long)__per_cpu_start)
>> +#define INVALID_PERCPU_AREA (0x8000000000000000UL - (long)__per_cpu_start)
>>  #define PERCPU_ORDER get_order_from_bytes(__per_cpu_data_end - 
>> __per_cpu_start)
> 
> Hi Jan, should this be ULL?

Not really, no - we're 64-bit only. Furthermore the expression is
about addresses, which correspond to "unsigned long" in our world.

>> --- a/xen/include/public/arch-x86/xen-x86_64.h
>> +++ b/xen/include/public/arch-x86/xen-x86_64.h
>> @@ -53,10 +53,10 @@
>>  #define FLAT_USER_SS32 FLAT_RING3_SS32
>>  #define FLAT_USER_SS   FLAT_USER_SS64
>>  
>> -#define __HYPERVISOR_VIRT_START 0xFFFF800000000000
>> -#define __HYPERVISOR_VIRT_END   0xFFFF880000000000
>> -#define __MACH2PHYS_VIRT_START  0xFFFF800000000000
>> -#define __MACH2PHYS_VIRT_END    0xFFFF804000000000
>> +#define __HYPERVISOR_VIRT_START 0xFFFF800000000000U
>> +#define __HYPERVISOR_VIRT_END   0xFFFF880000000000U
>> +#define __MACH2PHYS_VIRT_START  0xFFFF800000000000U
>> +#define __MACH2PHYS_VIRT_END    0xFFFF804000000000U
> 
> Also here ULL?

UL yes, but as above I don't think ULL is good to use for addresses.
That said, things are a little less clear in the public headers: In
principle it could be a goal for them to be usable on foreign
architectures (say e.g. a 32-bit tool stack, or an analysis tool
which can be run without being on the original host architecture) as
well.

Furthermore, open-coded use of ULL would make this no-longer-C89.
If anything, it would then need to be UINT64_C(), yet even that would
grow our public header dependencies on C99-like infrastructure being
available (right now we only require certain stdint.h-like types;
Arm alone, interestingly, also uses PRIx64 and PRIu64, which may be
kind of unavoidable there).

Jan