Messages in this thread | | | From | Nick Desaulniers <> | Date | Tue, 23 Apr 2019 11:21:28 -0700 | Subject | Re: [PATCH v2 4/4] s390: fix clang -Wpointer-sign warnigns in boot code |
| |
On Tue, Apr 23, 2019 at 1:06 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Mon, Apr 22, 2019 at 7:52 PM 'Nick Desaulniers' via Clang Built > Linux <clang-built-linux@googlegroups.com> wrote: > > > > @@ -20,7 +20,7 @@ extern __u8 _ebc_tolower[256]; /* EBCDIC -> lowercase */ > > > extern __u8 _ebc_toupper[256]; /* EBCDIC -> uppercase */ > > > > > > static inline void > > > -codepage_convert(const __u8 *codepage, volatile __u8 * addr, unsigned long nr) > > > +codepage_convert(const __u8 *codepage, volatile char* addr, unsigned long nr) > > > { > > > if (nr-- <= 0) > > > return; > > > > There are many call sites of ASCEBC which is defined in terms of this > > function. Do they all use `char*`? grep shows an explicit cast to > > `unsigned char*` in drivers/s390/char/tape_std.c for example. > > Generally speaking, the kernel is full of Wpointer-sign warnings, that's why > this warning is disabled in the top-level Makefile by default. My patch fixes > the ones in the s390 boot code that is not built with those default flags, but > I made no attempt to fix the rest of the kernel.
Right, sorry, I forgot about that. This patch looks good to me. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > Fun fact: on most architectures, 'char' is signed, but on s390 and 32-bit > arm it is unsigned. The compiler treats 'char', 'unsigned char' and > 'signed char' > as three distinct types here for that reason.
I think I recall reading about that in: https://www.amazon.com/ARM-System-Developers-Guide-Architecture/dp/1558608745 (I'll try to dig it up and post what the explanation was).
-- Thanks, ~Nick Desaulniers
| |