|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] misra: add deviations of MISRA C Rule 5.5
On 28.07.2025 17:29, Dmytro Prokopchuk1 wrote:
>
>
> On 7/28/25 16:15, Jan Beulich wrote:
>> On 28.07.2025 14:28, Dmytro Prokopchuk1 wrote:
>>>
>>>
>>> On 7/28/25 13:59, Andrew Cooper wrote:
>>>> On 28/07/2025 11:38 am, Nicola Vetrini wrote:
>>>>> On 2025-07-28 11:36, Jan Beulich wrote:
>>>>>> On 25.07.2025 18:24, Dmytro Prokopchuk1 wrote:
>>>>>>> --- a/docs/misra/deviations.rst
>>>>>>> +++ b/docs/misra/deviations.rst
>>>>>>> @@ -142,6 +142,31 @@ Deviations related to MISRA C:2012 Rules:
>>>>>>> memmove.
>>>>>>> - Tagged as `deliberate` for ECLAIR.
>>>>>>>
>>>>>>> + * - R5.5
>>>>>>> + - Clashes between bitops functions and macros names are
>>>>>>> deliberate and are
>>>>>>> + needed for input validation and error handling, ensures that
>>>>>>> the size of
>>>>>>> + the object being pointed to by 'addr' meets the minimum
>>>>>>> requirements for
>>>>>>> + the bit operation, preventing unsafe operations on
>>>>>>> improperly sized data
>>>>>>> + types that could lead to undefined behavior or memory
>>>>>>> corruption.
>>>>>>> + The macros encapsulate this conditional logic into a single,
>>>>>>> reusable form;
>>>>>>> + which simplifies the code, avoids redundant function call.
>>>>>>> + - Specified macros should be ignored.
>>>>>>
>>>>>> At the risk of going too far with nitpicking: Who are "specified
>>>>>> macros" here? The
>>>>>> text doesn't mention any names. In fact, the way it's written it
>>>>>> could be taken to
>>>>>> mean all macros there, including any that are yet to be added. I
>>>>>> don't think such
>>>>>> is appropriate for a deviation.
>>>>>>
>>>>>
>>>>> I agree with Jan here. Either you make a single deviation record
>>>>> encompassing all deviated macros or you have one per deviation (e.g.,
>>>>> one for irq.h, one for grant_table.h and one for bitops.h) listing the
>>>>> macros that are considered. For bitops it might be a concern the
>>>>> actual functions going out of sync, but in that case you could just
>>>>> spell out the deviation and say "all pairs functions/macros in file
>>>>> <file> that are defined using the same identifier" or something similar.
>>>>
>>>> Honestly, while these examples might be deliberate, they're also bad code.
>>>>
>>>> I do not intent to let the bitops aliases survive the cleanup/fixes I
>>>> have planned, but I also don't have any idea when I'll get to that work.
>>>>
>>>> What we really want to express is "these are begrudgingly accepted in
>>>> the short term. don't copy this pattern, and if you can fix it, please
>>>> do".
>>>>
>>>> ~Andrew
>>>
>>> Hi Andrew!
>>>
>>> Perhaps I can try to fix these names clashes.
>>>
>>> For clarity.
>>> I would like to rename macros names with capital letters.
>>> Like this:
>>> -#define __test_and_change_bit(nr, addr) ({ \
>>> +#define TEST_AND_CHANGE_BIT(nr, addr) ({ \
>>> if ( bitop_bad_size(addr) ) __bitop_bad_size(); \
>>> __test_and_change_bit(nr, addr); \
>>> })
>>>
>>> Are you OK with such approach?
>>
>> And then change all use sites of the macro to those upper-case forms?
> Yes.
>> When everyone's used to using the lower-case ones?
> Well, user habits vs. Misra compliance, clear code.
> I like second one.
> Let me repeat.
> I can prepare patch (it will touch many places in code base), and let
> maintainers decide what to do with it.
>
> While patch with deviations will be like spare plan.
>
> Jan, Andrew,
> are you agree with this?
No, I object to the renaming you intend to do. I don't think it is a useful
use of anyone's time to make or review such a change.
Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |