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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.07.2023 10:04, Simone Ballarin wrote:
> Il giorno ven 7 lug 2023 alle ore 09:04 Jan Beulich <jbeulich@xxxxxxxx> ha
> scritto:
> 
>> On 07.07.2023 08:50, Simone Ballarin wrote:
>>> Il giorno gio 6 lug 2023 alle ore 18:22 Jan Beulich <jbeulich@xxxxxxxx>
>> ha
>>> scritto:
>>>
>>>> On 06.07.2023 18:08, Simone Ballarin wrote:
>>>>> Il giorno gio 6 lug 2023 alle ore 10:26 Jan Beulich <jbeulich@xxxxxxxx
>>>
>>>> ha
>>>>> scritto:
>>>>>
>>>>>> On 05.07.2023 17:26, Simone Ballarin wrote:
>>>>>>> --- a/xen/arch/x86/apic.c
>>>>>>> +++ b/xen/arch/x86/apic.c
>>>>>>> @@ -1211,7 +1211,7 @@ static void __init calibrate_APIC_clock(void)
>>>>>>>       * Setup the APIC counter to maximum. There is no way the lapic
>>>>>>>       * can underflow in the 100ms detection time frame.
>>>>>>>       */
>>>>>>> -    __setup_APIC_LVTT(0xffffffff);
>>>>>>> +    __setup_APIC_LVTT(0xffffffffU);
>>>>>>
>>>>>> While making the change less mechanical, we want to consider to switch
>>>>>> to ~0 in this and similar cases.
>>>>>>
>>>>>
>>>>> Changing ~0U is more than not mechanical: it is possibly dangerous.
>>>>> The resulting value could be different depending on the architecture,
>>>>> I prefer to not make such kind of changes in a MISRA-related patch.
>>>>
>>>> What do you mean by "depending on the architecture", when this is
>>>> x86-only code _and_ you can check what type parameter the called
>>>> function has?
>>>
>>> Ok, I will change these literals in ~0U in the next submission.
>>
>> Except that I specifically meant ~0, not ~0U. We mean "maximum value"
>> here, and at the call site it doesn't matter how wide the function
>> parameter's type is. If it was 64-bit, ~0U would not do what is wanted.
> 
> ~0 is not a MISRA-compliant solution since bitwise operations on signed
> integers have implementation-defined behavior. This solution definitively
> violates Rule 10.1.

So if we adopted that rule (we didn't so far), we'd have to e.g. change
all literal number shift counts to have U suffixes, no matter that
without the suffix it is still entirely obvious that the numbers are
unsigned? I'm afraid that'll face my opposition ...

Jan