> On 14 Jul 2023, at 15:20, Luca Fancellu <luca.fance...@arm.com> wrote:
>
>
>
>> On 14 Jul 2023, at 12:49, Nicola Vetrini <nicola.vetr...@bugseng.com> wrote:
>>
>> The macro 'testop' expands to a function that declares the local
>> variable 'oldbit', which is written before being set, but is such a
>> way that is not amenable to automatic checking.
>>
>> Therefore, a deviation comment, is introduced to document this situation.
>>
>> A similar reasoning applies to macro 'guest_testop'.
>>
>> Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com>
>> ---
>> docs/misra/safe.json | 16 ++++++++++++++++
>> xen/arch/arm/arm64/lib/bitops.c | 3 +++
>> xen/arch/arm/include/asm/guest_atomics.h | 3 +++
>> 3 files changed, 22 insertions(+)
>>
>> diff --git a/docs/misra/safe.json b/docs/misra/safe.json
>> index 244001f5be..4cf7cbf57b 100644
>> --- a/docs/misra/safe.json
>> +++ b/docs/misra/safe.json
>> @@ -20,6 +20,22 @@
>> },
>> {
>> "id": "SAF-2-safe",
>> + "analyser": {
>> + "eclair": "MC3R1.R9.1"
>> + },
>> + "name": "Rule 9.1: initializer not needed",
>> + "text": "The following local variables are possibly subject to
>> being read before being written, but code inspection ensured that the
>> control flow in the construct where they appear ensures that no such event
>> may happen."
>> + },
>> + {
>> + "id": "SAF-3-safe",
>> + "analyser": {
>> + "eclair": "MC3R1.R9.1"
>> + },
>> + "name": "Rule 9.1: initializer not needed",
>> + "text": "The following local variables are possibly subject to
>> being read before being written, but code inspection ensured that the
>> control flow in the construct where they appear ensures that no such event
>> may happen."
>> + },
>
> Since the rule and the justification are the same, you can declare only once
> and use the same tag on top of the offending lines, so /* SAF-2-safe
> MC3R1.R9.1 */,
> also, I remember some maintainers not happy about the misra rule being put
> after the tag, now I don’t recall who
Sorry, I see there was in a patch before a SAF-1-safe with the same
justification, so I suggest you use SAF-3-safe as tag and drop the new
justifications introduced here
>
>> + {
>> + "id": "SAF-4-safe",
>> "analyser": {},
>> "name": "Sentinel",
>> "text": "Next ID to be used"
>> diff --git a/xen/arch/arm/arm64/lib/bitops.c
>> b/xen/arch/arm/arm64/lib/bitops.c
>> index 20e3f3d6ce..e0728bb29d 100644
>> --- a/xen/arch/arm/arm64/lib/bitops.c
>> +++ b/xen/arch/arm/arm64/lib/bitops.c
>> @@ -114,8 +114,11 @@ bitop(change_bit, eor)
>> bitop(clear_bit, bic)
>> bitop(set_bit, orr)
>>
>> +/* SAF-2-safe MC3R1.R9.1 */
>> testop(test_and_change_bit, eor)
>> +/* SAF-2-safe MC3R1.R9.1 */
>> testop(test_and_clear_bit, bic)
>> +/* SAF-2-safe MC3R1.R9.1 */
>> testop(test_and_set_bit, orr)
>>
>> static always_inline bool int_clear_mask16(uint16_t mask, volatile uint16_t
>> *p,
>> diff --git a/xen/arch/arm/include/asm/guest_atomics.h
>> b/xen/arch/arm/include/asm/guest_atomics.h
>> index a1745f8613..9d8f8ec3a3 100644
>> --- a/xen/arch/arm/include/asm/guest_atomics.h
>> +++ b/xen/arch/arm/include/asm/guest_atomics.h
>> @@ -67,8 +67,11 @@ guest_bitop(change_bit)
>> /* test_bit does not use load-store atomic operations */
>> #define guest_test_bit(d, nr, p) ((void)(d), test_bit(nr, p))
>>
>> +/* SAF-3-safe MC3R1.R9.1 */
>> guest_testop(test_and_set_bit)
>> +/* SAF-3-safe MC3R1.R9.1 */
>> guest_testop(test_and_clear_bit)
>> +/* SAF-3-safe MC3R1.R9.1 */
>> guest_testop(test_and_change_bit)
>>
>> #undef guest_testop
>> --
>> 2.34.1
>>
>>
>
