Re: [PATCH] lib/strtoul: fix MISRA R10.2 violation

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 14 May 2024, Jan Beulich wrote:
> On 14.05.2024 02:32, Stefano Stabellini wrote:
> > Fix last violation of R10.2 by casting the result of toupper to plain
> > char. Note that we don't want to change toupper itself as it is a legacy
> > interface and it would cause more issues.
> 
> Can you point me at a single example where a new issue would arise? All
> places I've spotted (including tolower() uses) would appear to benefit
> from changing toupper() / tolower() themselves. Further, since they are
> both wrapper macros only anyway, if any concern remained, fiddling with
> the wrapper macros while leaving alone the underlying inline functions
> would allow any such use site to simply be switched to using the inline
> functions directly. As said, from looking at it I don't expect that
> would be necessary, so instead I'd rather hope that eventually we can
> do away with the wrapper macros, renaming the inline functions
> accordingly.

If we change __toupper to return a plain char, then there are a few
other things we need to change for consistency, see below. To be honest
I thought it would cause more problems. I am OK to go with that if you
all agree. (Nicola please have a look in case this introduces more
issues elsewhere.)


diff --git a/xen/include/xen/ctype.h b/xen/include/xen/ctype.h
index 6dec944a37..6a6854e01c 100644
--- a/xen/include/xen/ctype.h
+++ b/xen/include/xen/ctype.h
@@ -15,9 +15,9 @@
 #define _X     0x40    /* hex digit */
 #define _SP    0x80    /* hard space (0x20) */
 
-extern const unsigned char _ctype[];
+extern const char _ctype[];
 
-#define __ismask(x) (_ctype[(int)(unsigned char)(x)])
+#define __ismask(x) (_ctype[(int)(char)(x)])
 
 #define isalnum(c)     ((__ismask(c)&(_U|_L|_D)) != 0)
 #define isalpha(c)     ((__ismask(c)&(_U|_L)) != 0)
@@ -34,14 +34,14 @@ extern const unsigned char _ctype[];
 #define isascii(c) (((unsigned char)(c))<=0x7f)
 #define toascii(c) (((unsigned char)(c))&0x7f)
 
-static inline unsigned char __tolower(unsigned char c)
+static inline char __tolower(char c)
 {
        if (isupper(c))
                c -= 'A'-'a';
        return c;
 }
 
-static inline unsigned char __toupper(unsigned char c)
+static inline char __toupper(char c)
 {
        if (islower(c))
                c -= 'a'-'A';
diff --git a/xen/lib/ctype.c b/xen/lib/ctype.c
index 7b233a335f..27e5b61b1c 100644
--- a/xen/lib/ctype.c
+++ b/xen/lib/ctype.c
@@ -1,7 +1,7 @@
 #include <xen/ctype.h>
 
 /* for ctype.h */
-const unsigned char _ctype[] = {
+const char _ctype[] = {
     _C,_C,_C,_C,_C,_C,_C,_C,                        /* 0-7 */
     _C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C,         /* 8-15 */
     _C,_C,_C,_C,_C,_C,_C,_C,                        /* 16-23 */