Re: [XEN PATCH v2] xen/string: add missing parameter names

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.08.2023 09:55, Federico Serafini wrote:
> On 03/08/23 21:26, Stefano Stabellini wrote:
>> On Thu, 3 Aug 2023, Julien Grall wrote:
>>> On 03/08/2023 12:52, Luca Fancellu wrote:
>>>>> On 3 Aug 2023, at 12:46, Julien Grall <julien@xxxxxxx> wrote:
>>>>>
>>>>> Hi Luca,
>>>>>
>>>>> On 03/08/2023 11:28, Luca Fancellu wrote:
>>>>>>> On 3 Aug 2023, at 09:26, Federico Serafini
>>>>>>> <federico.serafini@xxxxxxxxxxx> wrote:
>>>>>>>
>>>>>>> Add missing parameter names to address violation of MISRA C:2012
>>>>>>> rule 8.2 ("Function types shall be in prototype form with named
>>>>>>> parameters").
>>>>>>>
>>>>>>> No functional changes.
>>>>>>>
>>>>>>> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
>>>>>>> ---
>>>>>>> Changes in v2:
>>>>>>>    - memset() adjusted.
>>>>>>> ---
>>>>>>> xen/include/xen/string.h | 42 ++++++++++++++++++++--------------------
>>>>>>> 1 file changed, 21 insertions(+), 21 deletions(-)
>>>>>>>
>>>>>>> +void *memset(void *s, int c, size_t count);
>>>>>>> +void *memcpy(void *dest, const void *src, size_t count);
>>>>>> There is a comment in arch/arm/rm32/lib/memcpy.S with this:
>>>>>> /* Prototype: void *memcpy(void *dest, const void *src, size_t n); */
>>>>>>> +void *memmove(void *dest, const void *src, size_t count);
>>>>>> There is a comment in arch/arm/rm32/lib/memmove.S with this:
>>>>>>    * Prototype: void *memmove(void *dest, const void *src, size_t n);
>>>>>>> +int memcmp(const void *cs, const void *ct, size_t count);
>>>>>>> +void *memchr(const void *s, int c, size_t n);
>>>>>>> +void *memchr_inv(const void *s, int c, size_t n);
>>>>>> @Stefano: would it make sense to remove it as part of this patch or
>>>>>> maybe not?
>>>>>
>>>>> They are a verbatim copy of the Linux code. So I would rather no touch it.
>>>>
>>>> Oh I see! Thank you for pointing that out, then I’m wondering if it’s there
>>>> a reason why we
>>>> are using ‘count’ instead of ’n’ as third parameter name, I know Stefano
>>>> suggested that, so
>>>> It’s just a curiosity. Maybe it’s for clarity?
>>>
>>> I guess because the generic implementation of memset (see xen/lib/memset.c) 
>>> is
>>> using 'count' rather than 'n'.
>>
>> Yep
>>
>>
>>> Given what Andrew said, I would say we should rename the parameter to 'n'.
>>
>> Yes, either way works. I was only trying to be consistent with
>> xen/lib/memset.c. It is also fine to change xen/lib/memset.c instead.
> 
> If you want to be consistent compared to the C99 Standard,
> then other parameter names need to be changed, for example all the `cs`
> and `ct` should become `s1` and `s2`, respectively.
> The same goes for `dest` and `src`.
> If you agree, I can propose a v3 that takes care of that.

Personally I'd prefer if we could limit code churn. Functions that need
touching anyway can certainly be brought in line with names the standard
uses (albeit I don't see a strong need for this). But function which
won't otherwise be touched could easily be left alone.

Jan