[PATCH] 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/ad15582787e675fadf92502f85041c3232749a99.1756112701.git.dmytro._5Fprokopchuk1@epam.com
There is a newer version of this series
automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
docs/misra/deviations.rst                        | 8 ++++++++
2 files changed, 14 insertions(+)
[PATCH] 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()', 'snprintf()',
'strlcpy()', 'strlcat()', as they return a value purely for convenience,
their primary functionality (e.g., memory or string operations) 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>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
 docs/misra/deviations.rst                        | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 7f3fd35a33..23f62ef646 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -575,6 +575,12 @@ 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()', 'snprintf()', 'strlcpy()', 'strlcat()',
+as they return a value purely for convenience, their primary functionality (e.g., memory manipulation or string operations)
+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||snprintf||strlcpy||strlcat))"}
+-doc_end
+
 #
 # Series 18.
 #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 2119066531..ec98366aa5 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -576,6 +576,14 @@ Deviations related to MISRA C:2012 Rules:
          - __builtin_memset()
          - cpumask_check()
 
+   * - R17.7
+     - It is safe to deviate functions like 'memcpy()', 'memset()', 'memmove()',
+       'snprintf()', 'strlcpy()', 'strlcat()', as they return a value purely for
+       convenience, their primary functionality (e.g., memory manipulation or
+       string operations) 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] misra: add deviation of Rule 17.7
Posted by Jan Beulich 2 months ago
On 25.08.2025 11:05, Dmytro Prokopchuk1 wrote:
> 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()', 'snprintf()',
> 'strlcpy()', 'strlcat()', as they return a value purely for convenience,
> their primary functionality (e.g., memory or string operations) remains
> unaffected, and their return values are generally non-critical and seldom
> relied upon. Update 'deviations.rst' file accordingly.

How come snprintf() is among this set? Its return value isn't quite just
for convenience, imo.

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

On 8/25/25 13:30, Jan Beulich wrote:
> On 25.08.2025 11:05, Dmytro Prokopchuk1 wrote:
>> 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()', 'snprintf()',
>> 'strlcpy()', 'strlcat()', as they return a value purely for convenience,
>> their primary functionality (e.g., memory or string operations) remains
>> unaffected, and their return values are generally non-critical and seldom
>> relied upon. Update 'deviations.rst' file accordingly.
> 
> How come snprintf() is among this set? Its return value isn't quite just
> for convenience, imo.
> 
> Jan

Yes, snprintf()'s return value isn't just for convenience. The deviation 
justification is primarily based on the fact that its return value is 
rarely used in the Xen source base. Most callers of snprintf() don't 
care about return value. So, snprintf() is in this list.

Maybe separate wording is required for the snprintf() ?

Dmytro.
Re: [PATCH] misra: add deviation of Rule 17.7
Posted by Jan Beulich 2 months ago
On 25.08.2025 12:58, Dmytro Prokopchuk1 wrote:
> On 8/25/25 13:30, Jan Beulich wrote:
>> On 25.08.2025 11:05, Dmytro Prokopchuk1 wrote:
>>> 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()', 'snprintf()',
>>> 'strlcpy()', 'strlcat()', as they return a value purely for convenience,
>>> their primary functionality (e.g., memory or string operations) remains
>>> unaffected, and their return values are generally non-critical and seldom
>>> relied upon. Update 'deviations.rst' file accordingly.
>>
>> How come snprintf() is among this set? Its return value isn't quite just
>> for convenience, imo.
> 
> Yes, snprintf()'s return value isn't just for convenience. The deviation 
> justification is primarily based on the fact that its return value is 
> rarely used in the Xen source base. Most callers of snprintf() don't 
> care about return value. So, snprintf() is in this list.
> 
> Maybe separate wording is required for the snprintf() ?

Minimally. Personally I don't think it should be deviated globally.

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

On 8/25/25 14:07, Jan Beulich wrote:
> On 25.08.2025 12:58, Dmytro Prokopchuk1 wrote:
>> On 8/25/25 13:30, Jan Beulich wrote:
>>> On 25.08.2025 11:05, Dmytro Prokopchuk1 wrote:
>>>> 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()', 'snprintf()',
>>>> 'strlcpy()', 'strlcat()', as they return a value purely for convenience,
>>>> their primary functionality (e.g., memory or string operations) remains
>>>> unaffected, and their return values are generally non-critical and seldom
>>>> relied upon. Update 'deviations.rst' file accordingly.
>>>
>>> How come snprintf() is among this set? Its return value isn't quite just
>>> for convenience, imo.
>>
>> Yes, snprintf()'s return value isn't just for convenience. The deviation
>> justification is primarily based on the fact that its return value is
>> rarely used in the Xen source base. Most callers of snprintf() don't
>> care about return value. So, snprintf() is in this list.
>>
>> Maybe separate wording is required for the snprintf() ?
> 
> Minimally. Personally I don't think it should be deviated globally.
> 
> Jan

There are approximately 230 instances of snprintf() being used without 
checking its return value (across ARM and x86) in around 20 different 
source files. Deviation each of them could be complicated.

Dmytro.
Re: [PATCH] misra: add deviation of Rule 17.7
Posted by Jan Beulich 2 months ago
On 25.08.2025 15:27, Dmytro Prokopchuk1 wrote:
> 
> 
> On 8/25/25 14:07, Jan Beulich wrote:
>> On 25.08.2025 12:58, Dmytro Prokopchuk1 wrote:
>>> On 8/25/25 13:30, Jan Beulich wrote:
>>>> On 25.08.2025 11:05, Dmytro Prokopchuk1 wrote:
>>>>> 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()', 'snprintf()',
>>>>> 'strlcpy()', 'strlcat()', as they return a value purely for convenience,
>>>>> their primary functionality (e.g., memory or string operations) remains
>>>>> unaffected, and their return values are generally non-critical and seldom
>>>>> relied upon. Update 'deviations.rst' file accordingly.
>>>>
>>>> How come snprintf() is among this set? Its return value isn't quite just
>>>> for convenience, imo.
>>>
>>> Yes, snprintf()'s return value isn't just for convenience. The deviation
>>> justification is primarily based on the fact that its return value is
>>> rarely used in the Xen source base. Most callers of snprintf() don't
>>> care about return value. So, snprintf() is in this list.
>>>
>>> Maybe separate wording is required for the snprintf() ?
>>
>> Minimally. Personally I don't think it should be deviated globally.
> 
> There are approximately 230 instances of snprintf() being used without 
> checking its return value (across ARM and x86) in around 20 different 
> source files. Deviation each of them could be complicated.

My grep yields somewhere between 50 and 60 hits in xen/, among them about 15
in xen/tools/kconfig/, which I expect we can ignore. I also didn't mean to
suggest to deviate them all individually. Some may actually want to use the
return value, and I wouldn't be surprised if this ended up fixing a bug or
two.

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

On 8/25/25 17:12, Jan Beulich wrote:
> On 25.08.2025 15:27, Dmytro Prokopchuk1 wrote:
>>
>>
>> On 8/25/25 14:07, Jan Beulich wrote:
>>> On 25.08.2025 12:58, Dmytro Prokopchuk1 wrote:
>>>> On 8/25/25 13:30, Jan Beulich wrote:
>>>>> On 25.08.2025 11:05, Dmytro Prokopchuk1 wrote:
>>>>>> 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()', 'snprintf()',
>>>>>> 'strlcpy()', 'strlcat()', as they return a value purely for convenience,
>>>>>> their primary functionality (e.g., memory or string operations) remains
>>>>>> unaffected, and their return values are generally non-critical and seldom
>>>>>> relied upon. Update 'deviations.rst' file accordingly.
>>>>>
>>>>> How come snprintf() is among this set? Its return value isn't quite just
>>>>> for convenience, imo.
>>>>
>>>> Yes, snprintf()'s return value isn't just for convenience. The deviation
>>>> justification is primarily based on the fact that its return value is
>>>> rarely used in the Xen source base. Most callers of snprintf() don't
>>>> care about return value. So, snprintf() is in this list.
>>>>
>>>> Maybe separate wording is required for the snprintf() ?
>>>
>>> Minimally. Personally I don't think it should be deviated globally.
>>
>> There are approximately 230 instances of snprintf() being used without
>> checking its return value (across ARM and x86) in around 20 different
>> source files. Deviation each of them could be complicated.
> 
> My grep yields somewhere between 50 and 60 hits in xen/, among them about 15
> in xen/tools/kconfig/, which I expect we can ignore. I also didn't mean to
> suggest to deviate them all individually. Some may actually want to use the
> return value, and I wouldn't be surprised if this ended up fixing a bug or
> two.
> 
> Jan

For memory-related functions (memcpy, memset, memmove), ignoring the 
return value is almost always safe, so I propose to leave these 
functions in the patch, remove snprintf, strlcpy, strlcat so far, and
precisely check usage of the last functions.

Is it OK?
Re: [PATCH] misra: add deviation of Rule 17.7
Posted by Jan Beulich 2 months ago
On 25.08.2025 19:33, Dmytro Prokopchuk1 wrote:
> 
> 
> On 8/25/25 17:12, Jan Beulich wrote:
>> On 25.08.2025 15:27, Dmytro Prokopchuk1 wrote:
>>>
>>>
>>> On 8/25/25 14:07, Jan Beulich wrote:
>>>> On 25.08.2025 12:58, Dmytro Prokopchuk1 wrote:
>>>>> On 8/25/25 13:30, Jan Beulich wrote:
>>>>>> On 25.08.2025 11:05, Dmytro Prokopchuk1 wrote:
>>>>>>> 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()', 'snprintf()',
>>>>>>> 'strlcpy()', 'strlcat()', as they return a value purely for convenience,
>>>>>>> their primary functionality (e.g., memory or string operations) remains
>>>>>>> unaffected, and their return values are generally non-critical and seldom
>>>>>>> relied upon. Update 'deviations.rst' file accordingly.
>>>>>>
>>>>>> How come snprintf() is among this set? Its return value isn't quite just
>>>>>> for convenience, imo.
>>>>>
>>>>> Yes, snprintf()'s return value isn't just for convenience. The deviation
>>>>> justification is primarily based on the fact that its return value is
>>>>> rarely used in the Xen source base. Most callers of snprintf() don't
>>>>> care about return value. So, snprintf() is in this list.
>>>>>
>>>>> Maybe separate wording is required for the snprintf() ?
>>>>
>>>> Minimally. Personally I don't think it should be deviated globally.
>>>
>>> There are approximately 230 instances of snprintf() being used without
>>> checking its return value (across ARM and x86) in around 20 different
>>> source files. Deviation each of them could be complicated.
>>
>> My grep yields somewhere between 50 and 60 hits in xen/, among them about 15
>> in xen/tools/kconfig/, which I expect we can ignore. I also didn't mean to
>> suggest to deviate them all individually. Some may actually want to use the
>> return value, and I wouldn't be surprised if this ended up fixing a bug or
>> two.
> 
> For memory-related functions (memcpy, memset, memmove), ignoring the 
> return value is almost always safe, so I propose to leave these 
> functions in the patch, remove snprintf, strlcpy, strlcat so far, and
> precisely check usage of the last functions.
> 
> Is it OK?

Fine with me, yes.

Jan