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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 4 Aug 2023, Jan Beulich wrote:
> 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.
 
+1