| Message ID | 16c5362b740ed66100b55b528881cb26c1430f15.1691050900.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v2] xen/string: add missing parameter names | expand |
> On 3 Aug 2023, at 09:26, Federico Serafini <federico.serafini@bugseng.com> 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@bugseng.com> > --- > Changes in v2: > - memset() adjusted. > --- > xen/include/xen/string.h | 42 ++++++++++++++++++++-------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h > index b4d2217a96..e91e3112e0 100644 > --- a/xen/include/xen/string.h > +++ b/xen/include/xen/string.h > @@ -12,27 +12,27 @@ > #define strncpy __xen_has_no_strncpy__ > #define strncat __xen_has_no_strncat__ > > -size_t strlcpy(char *, const char *, size_t); > -size_t strlcat(char *, const char *, size_t); > -int strcmp(const char *, const char *); > -int strncmp(const char *, const char *, size_t); > -int strcasecmp(const char *, const char *); > -int strncasecmp(const char *, const char *, size_t); > -char *strchr(const char *, int); > -char *strrchr(const char *, int); > -char *strstr(const char *, const char *); > -size_t strlen(const char *); > -size_t strnlen(const char *, size_t); > -char *strpbrk(const char *, const char *); > -char *strsep(char **, const char *); > -size_t strspn(const char *, const char *); > - > -void *memset(void *, int, size_t); > -void *memcpy(void *, const void *, size_t); > -void *memmove(void *, const void *, size_t); > -int memcmp(const void *, const void *, size_t); > -void *memchr(const void *, int, size_t); > -void *memchr_inv(const void *, int, size_t); > +size_t strlcpy(char *dest, const char *src, size_t size); > +size_t strlcat(char *dest, const char *src, size_t size); > +int strcmp(const char *cs, const char *ct); > +int strncmp(const char *cs, const char *ct, size_t count); > +int strcasecmp(const char *s1, const char *s2); > +int strncasecmp(const char *s1, const char *s2, size_t len); > +char *strchr(const char *s, int c); > +char *strrchr(const char *s, int c); > +char *strstr(const char *s1, const char *s2); > +size_t strlen(const char *s); > +size_t strnlen(const char *s, size_t count); > +char *strpbrk(const char *cs, const char *ct); > +char *strsep(char **s, const char *ct); > +size_t strspn(const char *s, const char *accept); > + > +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? Apart from that, the patch looks good to me: Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Hi Luca, On 03/08/2023 11:28, Luca Fancellu wrote: >> On 3 Aug 2023, at 09:26, Federico Serafini <federico.serafini@bugseng.com> 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@bugseng.com> >> --- >> Changes in v2: >> - memset() adjusted. >> --- >> xen/include/xen/string.h | 42 ++++++++++++++++++++-------------------- >> 1 file changed, 21 insertions(+), 21 deletions(-) >> >> diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h >> index b4d2217a96..e91e3112e0 100644 >> --- a/xen/include/xen/string.h >> +++ b/xen/include/xen/string.h >> @@ -12,27 +12,27 @@ >> #define strncpy __xen_has_no_strncpy__ >> #define strncat __xen_has_no_strncat__ >> >> -size_t strlcpy(char *, const char *, size_t); >> -size_t strlcat(char *, const char *, size_t); >> -int strcmp(const char *, const char *); >> -int strncmp(const char *, const char *, size_t); >> -int strcasecmp(const char *, const char *); >> -int strncasecmp(const char *, const char *, size_t); >> -char *strchr(const char *, int); >> -char *strrchr(const char *, int); >> -char *strstr(const char *, const char *); >> -size_t strlen(const char *); >> -size_t strnlen(const char *, size_t); >> -char *strpbrk(const char *, const char *); >> -char *strsep(char **, const char *); >> -size_t strspn(const char *, const char *); >> - >> -void *memset(void *, int, size_t); >> -void *memcpy(void *, const void *, size_t); >> -void *memmove(void *, const void *, size_t); >> -int memcmp(const void *, const void *, size_t); >> -void *memchr(const void *, int, size_t); >> -void *memchr_inv(const void *, int, size_t); >> +size_t strlcpy(char *dest, const char *src, size_t size); >> +size_t strlcat(char *dest, const char *src, size_t size); >> +int strcmp(const char *cs, const char *ct); >> +int strncmp(const char *cs, const char *ct, size_t count); >> +int strcasecmp(const char *s1, const char *s2); >> +int strncasecmp(const char *s1, const char *s2, size_t len); >> +char *strchr(const char *s, int c); >> +char *strrchr(const char *s, int c); >> +char *strstr(const char *s1, const char *s2); >> +size_t strlen(const char *s); >> +size_t strnlen(const char *s, size_t count); >> +char *strpbrk(const char *cs, const char *ct); >> +char *strsep(char **s, const char *ct); >> +size_t strspn(const char *s, const char *accept); >> + >> +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. Cheers,
> On 3 Aug 2023, at 12:46, Julien Grall <julien@xen.org> wrote: > > Hi Luca, > > On 03/08/2023 11:28, Luca Fancellu wrote: >>> On 3 Aug 2023, at 09:26, Federico Serafini <federico.serafini@bugseng.com> 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@bugseng.com> >>> --- >>> Changes in v2: >>> - memset() adjusted. >>> --- >>> xen/include/xen/string.h | 42 ++++++++++++++++++++-------------------- >>> 1 file changed, 21 insertions(+), 21 deletions(-) >>> >>> diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h >>> index b4d2217a96..e91e3112e0 100644 >>> --- a/xen/include/xen/string.h >>> +++ b/xen/include/xen/string.h >>> @@ -12,27 +12,27 @@ >>> #define strncpy __xen_has_no_strncpy__ >>> #define strncat __xen_has_no_strncat__ >>> >>> -size_t strlcpy(char *, const char *, size_t); >>> -size_t strlcat(char *, const char *, size_t); >>> -int strcmp(const char *, const char *); >>> -int strncmp(const char *, const char *, size_t); >>> -int strcasecmp(const char *, const char *); >>> -int strncasecmp(const char *, const char *, size_t); >>> -char *strchr(const char *, int); >>> -char *strrchr(const char *, int); >>> -char *strstr(const char *, const char *); >>> -size_t strlen(const char *); >>> -size_t strnlen(const char *, size_t); >>> -char *strpbrk(const char *, const char *); >>> -char *strsep(char **, const char *); >>> -size_t strspn(const char *, const char *); >>> - >>> -void *memset(void *, int, size_t); >>> -void *memcpy(void *, const void *, size_t); >>> -void *memmove(void *, const void *, size_t); >>> -int memcmp(const void *, const void *, size_t); >>> -void *memchr(const void *, int, size_t); >>> -void *memchr_inv(const void *, int, size_t); >>> +size_t strlcpy(char *dest, const char *src, size_t size); >>> +size_t strlcat(char *dest, const char *src, size_t size); >>> +int strcmp(const char *cs, const char *ct); >>> +int strncmp(const char *cs, const char *ct, size_t count); >>> +int strcasecmp(const char *s1, const char *s2); >>> +int strncasecmp(const char *s1, const char *s2, size_t len); >>> +char *strchr(const char *s, int c); >>> +char *strrchr(const char *s, int c); >>> +char *strstr(const char *s1, const char *s2); >>> +size_t strlen(const char *s); >>> +size_t strnlen(const char *s, size_t count); >>> +char *strpbrk(const char *cs, const char *ct); >>> +char *strsep(char **s, const char *ct); >>> +size_t strspn(const char *s, const char *accept); >>> + >>> +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? > > Cheers, > > -- > Julien Grall
On 03/08/2023 12:52 pm, Luca Fancellu wrote: > >> On 3 Aug 2023, at 12:46, Julien Grall <julien@xen.org> wrote: >> >> Hi Luca, >> >> On 03/08/2023 11:28, Luca Fancellu wrote: >>>> On 3 Aug 2023, at 09:26, Federico Serafini <federico.serafini@bugseng.com> 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@bugseng.com> >>>> --- >>>> Changes in v2: >>>> - memset() adjusted. >>>> --- >>>> xen/include/xen/string.h | 42 ++++++++++++++++++++-------------------- >>>> 1 file changed, 21 insertions(+), 21 deletions(-) >>>> >>>> diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h >>>> index b4d2217a96..e91e3112e0 100644 >>>> --- a/xen/include/xen/string.h >>>> +++ b/xen/include/xen/string.h >>>> @@ -12,27 +12,27 @@ >>>> #define strncpy __xen_has_no_strncpy__ >>>> #define strncat __xen_has_no_strncat__ >>>> >>>> -size_t strlcpy(char *, const char *, size_t); >>>> -size_t strlcat(char *, const char *, size_t); >>>> -int strcmp(const char *, const char *); >>>> -int strncmp(const char *, const char *, size_t); >>>> -int strcasecmp(const char *, const char *); >>>> -int strncasecmp(const char *, const char *, size_t); >>>> -char *strchr(const char *, int); >>>> -char *strrchr(const char *, int); >>>> -char *strstr(const char *, const char *); >>>> -size_t strlen(const char *); >>>> -size_t strnlen(const char *, size_t); >>>> -char *strpbrk(const char *, const char *); >>>> -char *strsep(char **, const char *); >>>> -size_t strspn(const char *, const char *); >>>> - >>>> -void *memset(void *, int, size_t); >>>> -void *memcpy(void *, const void *, size_t); >>>> -void *memmove(void *, const void *, size_t); >>>> -int memcmp(const void *, const void *, size_t); >>>> -void *memchr(const void *, int, size_t); >>>> -void *memchr_inv(const void *, int, size_t); >>>> +size_t strlcpy(char *dest, const char *src, size_t size); >>>> +size_t strlcat(char *dest, const char *src, size_t size); >>>> +int strcmp(const char *cs, const char *ct); >>>> +int strncmp(const char *cs, const char *ct, size_t count); >>>> +int strcasecmp(const char *s1, const char *s2); >>>> +int strncasecmp(const char *s1, const char *s2, size_t len); >>>> +char *strchr(const char *s, int c); >>>> +char *strrchr(const char *s, int c); >>>> +char *strstr(const char *s1, const char *s2); >>>> +size_t strlen(const char *s); >>>> +size_t strnlen(const char *s, size_t count); >>>> +char *strpbrk(const char *cs, const char *ct); >>>> +char *strsep(char **s, const char *ct); >>>> +size_t strspn(const char *s, const char *accept); >>>> + >>>> +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? 'n' is what the parameter is called in the C spec. ~Andrew
Hi, On 03/08/2023 12:52, Luca Fancellu wrote: >> On 3 Aug 2023, at 12:46, Julien Grall <julien@xen.org> wrote: >> >> Hi Luca, >> >> On 03/08/2023 11:28, Luca Fancellu wrote: >>>> On 3 Aug 2023, at 09:26, Federico Serafini <federico.serafini@bugseng.com> 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@bugseng.com> >>>> --- >>>> Changes in v2: >>>> - memset() adjusted. >>>> --- >>>> xen/include/xen/string.h | 42 ++++++++++++++++++++-------------------- >>>> 1 file changed, 21 insertions(+), 21 deletions(-) >>>> >>>> diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h >>>> index b4d2217a96..e91e3112e0 100644 >>>> --- a/xen/include/xen/string.h >>>> +++ b/xen/include/xen/string.h >>>> @@ -12,27 +12,27 @@ >>>> #define strncpy __xen_has_no_strncpy__ >>>> #define strncat __xen_has_no_strncat__ >>>> >>>> -size_t strlcpy(char *, const char *, size_t); >>>> -size_t strlcat(char *, const char *, size_t); >>>> -int strcmp(const char *, const char *); >>>> -int strncmp(const char *, const char *, size_t); >>>> -int strcasecmp(const char *, const char *); >>>> -int strncasecmp(const char *, const char *, size_t); >>>> -char *strchr(const char *, int); >>>> -char *strrchr(const char *, int); >>>> -char *strstr(const char *, const char *); >>>> -size_t strlen(const char *); >>>> -size_t strnlen(const char *, size_t); >>>> -char *strpbrk(const char *, const char *); >>>> -char *strsep(char **, const char *); >>>> -size_t strspn(const char *, const char *); >>>> - >>>> -void *memset(void *, int, size_t); >>>> -void *memcpy(void *, const void *, size_t); >>>> -void *memmove(void *, const void *, size_t); >>>> -int memcmp(const void *, const void *, size_t); >>>> -void *memchr(const void *, int, size_t); >>>> -void *memchr_inv(const void *, int, size_t); >>>> +size_t strlcpy(char *dest, const char *src, size_t size); >>>> +size_t strlcat(char *dest, const char *src, size_t size); >>>> +int strcmp(const char *cs, const char *ct); >>>> +int strncmp(const char *cs, const char *ct, size_t count); >>>> +int strcasecmp(const char *s1, const char *s2); >>>> +int strncasecmp(const char *s1, const char *s2, size_t len); >>>> +char *strchr(const char *s, int c); >>>> +char *strrchr(const char *s, int c); >>>> +char *strstr(const char *s1, const char *s2); >>>> +size_t strlen(const char *s); >>>> +size_t strnlen(const char *s, size_t count); >>>> +char *strpbrk(const char *cs, const char *ct); >>>> +char *strsep(char **s, const char *ct); >>>> +size_t strspn(const char *s, const char *accept); >>>> + >>>> +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'. Given what Andrew said, I would say we should rename the parameter to 'n'. Cheers,
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@xen.org> wrote: > > > > > > Hi Luca, > > > > > > On 03/08/2023 11:28, Luca Fancellu wrote: > > > > > On 3 Aug 2023, at 09:26, Federico Serafini > > > > > <federico.serafini@bugseng.com> 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@bugseng.com> > > > > > --- > > > > > Changes in v2: > > > > > - memset() adjusted. > > > > > --- > > > > > xen/include/xen/string.h | 42 ++++++++++++++++++++-------------------- > > > > > 1 file changed, 21 insertions(+), 21 deletions(-) > > > > > > > > > > diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h > > > > > index b4d2217a96..e91e3112e0 100644 > > > > > --- a/xen/include/xen/string.h > > > > > +++ b/xen/include/xen/string.h > > > > > @@ -12,27 +12,27 @@ > > > > > #define strncpy __xen_has_no_strncpy__ > > > > > #define strncat __xen_has_no_strncat__ > > > > > > > > > > -size_t strlcpy(char *, const char *, size_t); > > > > > -size_t strlcat(char *, const char *, size_t); > > > > > -int strcmp(const char *, const char *); > > > > > -int strncmp(const char *, const char *, size_t); > > > > > -int strcasecmp(const char *, const char *); > > > > > -int strncasecmp(const char *, const char *, size_t); > > > > > -char *strchr(const char *, int); > > > > > -char *strrchr(const char *, int); > > > > > -char *strstr(const char *, const char *); > > > > > -size_t strlen(const char *); > > > > > -size_t strnlen(const char *, size_t); > > > > > -char *strpbrk(const char *, const char *); > > > > > -char *strsep(char **, const char *); > > > > > -size_t strspn(const char *, const char *); > > > > > - > > > > > -void *memset(void *, int, size_t); > > > > > -void *memcpy(void *, const void *, size_t); > > > > > -void *memmove(void *, const void *, size_t); > > > > > -int memcmp(const void *, const void *, size_t); > > > > > -void *memchr(const void *, int, size_t); > > > > > -void *memchr_inv(const void *, int, size_t); > > > > > +size_t strlcpy(char *dest, const char *src, size_t size); > > > > > +size_t strlcat(char *dest, const char *src, size_t size); > > > > > +int strcmp(const char *cs, const char *ct); > > > > > +int strncmp(const char *cs, const char *ct, size_t count); > > > > > +int strcasecmp(const char *s1, const char *s2); > > > > > +int strncasecmp(const char *s1, const char *s2, size_t len); > > > > > +char *strchr(const char *s, int c); > > > > > +char *strrchr(const char *s, int c); > > > > > +char *strstr(const char *s1, const char *s2); > > > > > +size_t strlen(const char *s); > > > > > +size_t strnlen(const char *s, size_t count); > > > > > +char *strpbrk(const char *cs, const char *ct); > > > > > +char *strsep(char **s, const char *ct); > > > > > +size_t strspn(const char *s, const char *accept); > > > > > + > > > > > +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.
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@xen.org> wrote: >>>> >>>> Hi Luca, >>>> >>>> On 03/08/2023 11:28, Luca Fancellu wrote: >>>>>> On 3 Aug 2023, at 09:26, Federico Serafini >>>>>> <federico.serafini@bugseng.com> 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@bugseng.com> >>>>>> --- >>>>>> 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.
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@xen.org> wrote: >>>>> >>>>> Hi Luca, >>>>> >>>>> On 03/08/2023 11:28, Luca Fancellu wrote: >>>>>>> On 3 Aug 2023, at 09:26, Federico Serafini >>>>>>> <federico.serafini@bugseng.com> 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@bugseng.com> >>>>>>> --- >>>>>>> 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
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@xen.org> wrote: > >>>>> > >>>>> Hi Luca, > >>>>> > >>>>> On 03/08/2023 11:28, Luca Fancellu wrote: > >>>>>>> On 3 Aug 2023, at 09:26, Federico Serafini > >>>>>>> <federico.serafini@bugseng.com> 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@bugseng.com> > >>>>>>> --- > >>>>>>> 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
diff --git a/xen/include/xen/string.h b/xen/include/xen/string.h index b4d2217a96..e91e3112e0 100644 --- a/xen/include/xen/string.h +++ b/xen/include/xen/string.h @@ -12,27 +12,27 @@ #define strncpy __xen_has_no_strncpy__ #define strncat __xen_has_no_strncat__ -size_t strlcpy(char *, const char *, size_t); -size_t strlcat(char *, const char *, size_t); -int strcmp(const char *, const char *); -int strncmp(const char *, const char *, size_t); -int strcasecmp(const char *, const char *); -int strncasecmp(const char *, const char *, size_t); -char *strchr(const char *, int); -char *strrchr(const char *, int); -char *strstr(const char *, const char *); -size_t strlen(const char *); -size_t strnlen(const char *, size_t); -char *strpbrk(const char *, const char *); -char *strsep(char **, const char *); -size_t strspn(const char *, const char *); - -void *memset(void *, int, size_t); -void *memcpy(void *, const void *, size_t); -void *memmove(void *, const void *, size_t); -int memcmp(const void *, const void *, size_t); -void *memchr(const void *, int, size_t); -void *memchr_inv(const void *, int, size_t); +size_t strlcpy(char *dest, const char *src, size_t size); +size_t strlcat(char *dest, const char *src, size_t size); +int strcmp(const char *cs, const char *ct); +int strncmp(const char *cs, const char *ct, size_t count); +int strcasecmp(const char *s1, const char *s2); +int strncasecmp(const char *s1, const char *s2, size_t len); +char *strchr(const char *s, int c); +char *strrchr(const char *s, int c); +char *strstr(const char *s1, const char *s2); +size_t strlen(const char *s); +size_t strnlen(const char *s, size_t count); +char *strpbrk(const char *cs, const char *ct); +char *strsep(char **s, const char *ct); +size_t strspn(const char *s, const char *accept); + +void *memset(void *s, int c, size_t count); +void *memcpy(void *dest, const void *src, size_t count); +void *memmove(void *dest, const void *src, size_t count); +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); #include <asm/string.h>
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@bugseng.com> --- Changes in v2: - memset() adjusted. --- xen/include/xen/string.h | 42 ++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-)