[PATCH v2] misra: add deviation of Rule 17.7

Dmytro Prokopchuk1 posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/812b78119cee801662a31d39b556cb453aa69508.1756192362.git.dmytro._5Fprokopchuk1@epam.com
There is a newer version of this series
automation/eclair_analysis/ECLAIR/deviations.ecl | 5 +++++
docs/misra/deviations.rst                        | 7 +++++++
2 files changed, 12 insertions(+)
[PATCH v2] misra: add deviation of Rule 17.7
Posted by Dmytro Prokopchuk1 2 months ago
MISRA C Rule 17.7 states: "The value returned by a function having
non-void return type shall be used."

Deviate functions like 'memcpy()', 'memset()', 'memmove()', as they
return a value purely for convenience, their primary functionality
(memory manipulation) remains unaffected, and their return values
are generally non-critical and seldom relied upon.

Update 'deviations.rst' file accordingly. No functional changes.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
Changes in v2:
- removed snprintf(), strlcpy(), strlcat() from the patch scope

Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2003537146

Link to v1:
https://patchew.org/Xen/ad15582787e675fadf92502f85041c3232749a99.1756112701.git.dmytro._5Fprokopchuk1@epam.com/
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 5 +++++
 docs/misra/deviations.rst                        | 7 +++++++
 2 files changed, 12 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 7f3fd35a33..8335af1bce 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -575,6 +575,11 @@ safe."
 -config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(__builtin_memcpy||__builtin_memmove||__builtin_memset||cpumask_check))"}
 -doc_end
 
+-doc_begin="It is safe to deviate functions like 'memcpy()', 'memset()', 'memmove()', as they return a value purely for convenience,
+their primary functionality (memory manipulation) remains unaffected, and their return values are generally non-critical and seldom relied upon."
+-config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(memcpy||memset||memmove))"}
+-doc_end
+
 #
 # Series 18.
 #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 2119066531..5ee97f41e8 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -576,6 +576,13 @@ Deviations related to MISRA C:2012 Rules:
          - __builtin_memset()
          - cpumask_check()
 
+   * - R17.7
+     - It is safe to deviate functions like 'memcpy()', 'memset()', 'memmove()',
+       as they return a value purely for convenience, their primary functionality
+       (memory manipulation) remains unaffected, and their return values are
+       generally non-critical and seldom relied upon.
+     - Tagged as `safe` for ECLAIR.
+
    * - R18.2
      - Subtractions between pointers where at least one of the operand is a
        pointer to a symbol defined by the linker are safe.
-- 
2.43.0
Re: [PATCH v2] misra: add deviation of Rule 17.7
Posted by Jan Beulich 2 months ago
On 26.08.2025 09:36, Dmytro Prokopchuk1 wrote:
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -575,6 +575,11 @@ safe."
>  -config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(__builtin_memcpy||__builtin_memmove||__builtin_memset||cpumask_check))"}
>  -doc_end
>  
> +-doc_begin="It is safe to deviate functions like 'memcpy()', 'memset()', 'memmove()', as they return a value purely for convenience,
> +their primary functionality (memory manipulation) remains unaffected, and their return values are generally non-critical and seldom relied upon."
> +-config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(memcpy||memset||memmove))"}
> +-doc_end
> +
>  #
>  # Series 18.
>  #
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -576,6 +576,13 @@ Deviations related to MISRA C:2012 Rules:
>           - __builtin_memset()
>           - cpumask_check()
>  
> +   * - R17.7
> +     - It is safe to deviate functions like 'memcpy()', 'memset()', 'memmove()',
> +       as they return a value purely for convenience, their primary functionality
> +       (memory manipulation) remains unaffected, and their return values are
> +       generally non-critical and seldom relied upon.
> +     - Tagged as `safe` for ECLAIR.

I realize I may be overly nitpicky here, but in files named deviations.* I find it
odd to read "It is safe to deviate ...". I further find the use of "like" odd when
you enumerate the complete set anyway.

I wonder whether the deviation wants generalizing anyway: Informational return
values are generally okay to ignore. That is, the Eclair configuration would be
limited to the three functions for now, but the text / comment could already be
broader. Then, for example, open-coded uses of the corresponding builtin functions
would also be covered right away.

Jan
Re: [PATCH v2] misra: add deviation of Rule 17.7
Posted by Nicola Vetrini 2 months ago
On 2025-08-26 09:45, Jan Beulich wrote:
> On 26.08.2025 09:36, Dmytro Prokopchuk1 wrote:
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -575,6 +575,11 @@ safe."
>>  -config=MC3A2.R17.7,calls+={safe, "any()", 
>> "decl(name(__builtin_memcpy||__builtin_memmove||__builtin_memset||cpumask_check))"}
>>  -doc_end
>> 
>> +-doc_begin="It is safe to deviate functions like 'memcpy()', 
>> 'memset()', 'memmove()', as they return a value purely for 
>> convenience,
>> +their primary functionality (memory manipulation) remains unaffected, 
>> and their return values are generally non-critical and seldom relied 
>> upon."
>> +-config=MC3A2.R17.7,calls+={safe, "any()", 
>> "decl(name(memcpy||memset||memmove))"}
>> +-doc_end
>> +
>>  #
>>  # Series 18.
>>  #
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -576,6 +576,13 @@ Deviations related to MISRA C:2012 Rules:
>>           - __builtin_memset()
>>           - cpumask_check()
>> 
>> +   * - R17.7
>> +     - It is safe to deviate functions like 'memcpy()', 'memset()', 
>> 'memmove()',
>> +       as they return a value purely for convenience, their primary 
>> functionality
>> +       (memory manipulation) remains unaffected, and their return 
>> values are
>> +       generally non-critical and seldom relied upon.
>> +     - Tagged as `safe` for ECLAIR.
> 
> I realize I may be overly nitpicky here, but in files named 
> deviations.* I find it
> odd to read "It is safe to deviate ...". I further find the use of 
> "like" odd when
> you enumerate the complete set anyway.
> 
> I wonder whether the deviation wants generalizing anyway: Informational 
> return
> values are generally okay to ignore. That is, the Eclair configuration 
> would be
> limited to the three functions for now, but the text / comment could 
> already be
> broader. Then, for example, open-coded uses of the corresponding 
> builtin functions
> would also be covered right away.
> 

While I understand the pragmatic reasoning, from a MISRA compliance 
standpoint it would be better not to make the written justification and 
the actual deviation diverge, and then wide both the ECLAIR 
configuration and its justification suitably once new cases want to be 
deviated. Other than that,

Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
Re: [PATCH v2] misra: add deviation of Rule 17.7
Posted by Dmytro Prokopchuk1 2 months ago

On 8/26/25 10:56, Nicola Vetrini wrote:
> On 2025-08-26 09:45, Jan Beulich wrote:
>> On 26.08.2025 09:36, Dmytro Prokopchuk1 wrote:
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -575,6 +575,11 @@ safe."
>>>  -config=MC3A2.R17.7,calls+={safe, "any()", 
>>> "decl(name(__builtin_memcpy||__builtin_memmove||__builtin_memset|| 
>>> cpumask_check))"}
>>>  -doc_end
>>>
>>> +-doc_begin="It is safe to deviate functions like 'memcpy()', 
>>> 'memset()', 'memmove()', as they return a value purely for convenience,
>>> +their primary functionality (memory manipulation) remains 
>>> unaffected, and their return values are generally non-critical and 
>>> seldom relied upon."
>>> +-config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(memcpy|| 
>>> memset||memmove))"}
>>> +-doc_end
>>> +
>>>  #
>>>  # Series 18.
>>>  #
>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -576,6 +576,13 @@ Deviations related to MISRA C:2012 Rules:
>>>           - __builtin_memset()
>>>           - cpumask_check()
>>>
>>> +   * - R17.7
>>> +     - It is safe to deviate functions like 'memcpy()', 'memset()', 
>>> 'memmove()',
>>> +       as they return a value purely for convenience, their primary 
>>> functionality
>>> +       (memory manipulation) remains unaffected, and their return 
>>> values are
>>> +       generally non-critical and seldom relied upon.
>>> +     - Tagged as `safe` for ECLAIR.
>>
>> I realize I may be overly nitpicky here, but in files named 
>> deviations.* I find it
>> odd to read "It is safe to deviate ...". I further find the use of 
>> "like" odd when
>> you enumerate the complete set anyway.

Updated wording (probably for the v3, if it's fine):

The functions 'memcpy()', 'memset()', and 'memmove()' return values 
primarily for convenience.
The core functionality of these functions (memory manipulation) remains 
unaffected, and their return values
are generally non-critical and seldom relied upon. Therefore, deviations 
from this rule regarding their use
can be considered safe.

Dmytro.

>>
>> I wonder whether the deviation wants generalizing anyway: 
>> Informational return
>> values are generally okay to ignore. That is, the Eclair configuration 
>> would be
>> limited to the three functions for now, but the text / comment could 
>> already be
>> broader. Then, for example, open-coded uses of the corresponding 
>> builtin functions
>> would also be covered right away.
>>
> 
> While I understand the pragmatic reasoning, from a MISRA compliance 
> standpoint it would be better not to make the written justification and 
> the actual deviation diverge, and then wide both the ECLAIR 
> configuration and its justification suitably once new cases want to be 
> deviated. Other than that,
> 
> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
Re: [PATCH v2] misra: add deviation of Rule 17.7
Posted by Nicola Vetrini 2 months ago
On 2025-08-26 15:14, Dmytro Prokopchuk1 wrote:
> On 8/26/25 10:56, Nicola Vetrini wrote:
>> On 2025-08-26 09:45, Jan Beulich wrote:
>>> On 26.08.2025 09:36, Dmytro Prokopchuk1 wrote:
>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> @@ -575,6 +575,11 @@ safe."
>>>>  -config=MC3A2.R17.7,calls+={safe, "any()",
>>>> "decl(name(__builtin_memcpy||__builtin_memmove||__builtin_memset||
>>>> cpumask_check))"}
>>>>  -doc_end
>>>> 
>>>> +-doc_begin="It is safe to deviate functions like 'memcpy()',
>>>> 'memset()', 'memmove()', as they return a value purely for 
>>>> convenience,
>>>> +their primary functionality (memory manipulation) remains
>>>> unaffected, and their return values are generally non-critical and
>>>> seldom relied upon."
>>>> +-config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(memcpy||
>>>> memset||memmove))"}
>>>> +-doc_end
>>>> +
>>>>  #
>>>>  # Series 18.
>>>>  #
>>>> --- a/docs/misra/deviations.rst
>>>> +++ b/docs/misra/deviations.rst
>>>> @@ -576,6 +576,13 @@ Deviations related to MISRA C:2012 Rules:
>>>>           - __builtin_memset()
>>>>           - cpumask_check()
>>>> 
>>>> +   * - R17.7
>>>> +     - It is safe to deviate functions like 'memcpy()', 'memset()',
>>>> 'memmove()',
>>>> +       as they return a value purely for convenience, their primary
>>>> functionality
>>>> +       (memory manipulation) remains unaffected, and their return
>>>> values are
>>>> +       generally non-critical and seldom relied upon.
>>>> +     - Tagged as `safe` for ECLAIR.
>>> 
>>> I realize I may be overly nitpicky here, but in files named
>>> deviations.* I find it
>>> odd to read "It is safe to deviate ...". I further find the use of
>>> "like" odd when
>>> you enumerate the complete set anyway.
> 
> Updated wording (probably for the v3, if it's fine):
> 
> The functions 'memcpy()', 'memset()', and 'memmove()' return values
> primarily for convenience.
> The core functionality of these functions (memory manipulation) remains
> unaffected, and their return values
> are generally non-critical and seldom relied upon. Therefore, 
> deviations
> from this rule regarding their use
> can be considered safe.
> 

The last sentence reads a little strangely. I would write: "Therefore, 
violations of this rule due to these functions are deemed safe."

> Dmytro.
> 
>>> 
>>> I wonder whether the deviation wants generalizing anyway:
>>> Informational return
>>> values are generally okay to ignore. That is, the Eclair 
>>> configuration
>>> would be
>>> limited to the three functions for now, but the text / comment could
>>> already be
>>> broader. Then, for example, open-coded uses of the corresponding
>>> builtin functions
>>> would also be covered right away.
>>> 
>> 
>> While I understand the pragmatic reasoning, from a MISRA compliance
>> standpoint it would be better not to make the written justification 
>> and
>> the actual deviation diverge, and then wide both the ECLAIR
>> configuration and its justification suitably once new cases want to be
>> deviated. Other than that,
>> 
>> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> 

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH v2] misra: add deviation of Rule 17.7
Posted by Dmytro Prokopchuk1 2 months ago

On 8/26/25 10:45, Jan Beulich wrote:
> On 26.08.2025 09:36, Dmytro Prokopchuk1 wrote:
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -575,6 +575,11 @@ safe."
>>   -config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(__builtin_memcpy||__builtin_memmove||__builtin_memset||cpumask_check))"}
>>   -doc_end
>>   
>> +-doc_begin="It is safe to deviate functions like 'memcpy()', 'memset()', 'memmove()', as they return a value purely for convenience,
>> +their primary functionality (memory manipulation) remains unaffected, and their return values are generally non-critical and seldom relied upon."
>> +-config=MC3A2.R17.7,calls+={safe, "any()", "decl(name(memcpy||memset||memmove))"}
>> +-doc_end
>> +
>>   #
>>   # Series 18.
>>   #
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -576,6 +576,13 @@ Deviations related to MISRA C:2012 Rules:
>>            - __builtin_memset()
>>            - cpumask_check()
>>   
>> +   * - R17.7
>> +     - It is safe to deviate functions like 'memcpy()', 'memset()', 'memmove()',
>> +       as they return a value purely for convenience, their primary functionality
>> +       (memory manipulation) remains unaffected, and their return values are
>> +       generally non-critical and seldom relied upon.
>> +     - Tagged as `safe` for ECLAIR.
> 
> I realize I may be overly nitpicky here, but in files named deviations.* I find it
> odd to read "It is safe to deviate ...". I further find the use of "like" odd when
> you enumerate the complete set anyway.
> 
> I wonder whether the deviation wants generalizing anyway: Informational return
> values are generally okay to ignore. That is, the Eclair configuration would be
> limited to the three functions for now, but the text / comment could already be
> broader. Then, for example, open-coded uses of the corresponding builtin functions
> would also be covered right away.
> 
> Jan

Yes, fully agree with you.
I'll update it.
Thanks.

Dmytro.