[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [XEN PATCH v2 12/13] xen/x86: fix violations of MISRA C:2012 Rule 7.2
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
- Date: Fri, 7 Jul 2023 10:04:08 +0200
- Cc: consulting@xxxxxxxxxxx, Gianluca Luparini <gianluca.luparini@xxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Xenia Ragiadakou <Xenia.Ragiadakou@xxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Fri, 07 Jul 2023 08:04:36 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
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.
Jan
~0 is not a MISRA-compliant solution since bitwise operations on signed integers have implementation-defined behavior. This solution definitively violates Rule 10.1. As you said ~0 is different than ~0U, 0xffffffffU, and 0xffffffff, so using ~0 means changing the semantics of the code: this is not the aim of the patch.
|
