[PATCH] s390/time: Fix clk type in get_tod_clock

[Nathan Chancellor]

Clang warns:

In file included from ../arch/s390/boot/startup.c:3:
In file included from ../include/linux/elf.h:5:
In file included from ../arch/s390/include/asm/elf.h:132:
In file included from ../include/linux/compat.h:10:
In file included from ../include/linux/time.h:74:
In file included from ../include/linux/time32.h:13:
In file included from ../include/linux/timex.h:65:
../arch/s390/include/asm/timex.h:160:20: warning: passing 'unsigned char
[16]' to parameter of type 'char *' converts between pointers to integer
types with different sign [-Wpointer-sign]
get_tod_clock_ext(clk);
^~~
../arch/s390/include/asm/timex.h:149:44: note: passing argument to
parameter 'clk' here
static inline void get_tod_clock_ext(char *clk)
^

Change clk's type to just be char so that it matches what happens in
get_tod_clock_ext.

Fixes: 57b28f66316d ("[S390] s390_hypfs: Add new attributes")
Link: https://github.com/ClangBuiltLinux/linux/issues/861
Signed-off-by: Nathan Chancellor <natecha...@gmail.com>
---

Alternatively, changing the clk type in get_tod_clock_ext to unsigned
which is what it was in the early 2000s.

arch/s390/include/asm/timex.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/include/asm/timex.h b/arch/s390/include/asm/timex.h
index 670f14a228e5..6bf3a45ccfec 100644
--- a/arch/s390/include/asm/timex.h
+++ b/arch/s390/include/asm/timex.h
@@ -155,7 +155,7 @@ static inline void get_tod_clock_ext(char *clk)

static inline unsigned long long get_tod_clock(void)
{
- unsigned char clk[STORE_CLOCK_EXT_SIZE];
+ char clk[STORE_CLOCK_EXT_SIZE];

get_tod_clock_ext(clk);
return *((unsigned long long *)&clk[1]);
--
2.25.0

[Nick Desaulniers]

On Sat, Feb 8, 2020 at 3:10 PM Nathan Chancellor
<natecha...@gmail.com> wrote:
>
> Clang warns:
>
> In file included from ../arch/s390/boot/startup.c:3:
> In file included from ../include/linux/elf.h:5:
> In file included from ../arch/s390/include/asm/elf.h:132:
> In file included from ../include/linux/compat.h:10:
> In file included from ../include/linux/time.h:74:
> In file included from ../include/linux/time32.h:13:
> In file included from ../include/linux/timex.h:65:
> ../arch/s390/include/asm/timex.h:160:20: warning: passing 'unsigned char
> [16]' to parameter of type 'char *' converts between pointers to integer
> types with different sign [-Wpointer-sign]
> get_tod_clock_ext(clk);
> ^~~
> ../arch/s390/include/asm/timex.h:149:44: note: passing argument to
> parameter 'clk' here
> static inline void get_tod_clock_ext(char *clk)
> ^
>
> Change clk's type to just be char so that it matches what happens in
> get_tod_clock_ext.
>
> Fixes: 57b28f66316d ("[S390] s390_hypfs: Add new attributes")
> Link: https://github.com/ClangBuiltLinux/linux/issues/861
> Signed-off-by: Nathan Chancellor <natecha...@gmail.com>

First time I've seen a `typedef` in a function. I wonder if that makes
its definition have function scope? (re: get_tod_clock_ext())

> ---
>
> Alternatively, changing the clk type in get_tod_clock_ext to unsigned
> which is what it was in the early 2000s.

Yeah, it doesn't really matter for this case, it looks like. Either way,

Reviewed-by: Nick Desaulniers <ndesau...@google.com>

>
> arch/s390/include/asm/timex.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/include/asm/timex.h b/arch/s390/include/asm/timex.h
> index 670f14a228e5..6bf3a45ccfec 100644
> --- a/arch/s390/include/asm/timex.h
> +++ b/arch/s390/include/asm/timex.h
> @@ -155,7 +155,7 @@ static inline void get_tod_clock_ext(char *clk)
>
> static inline unsigned long long get_tod_clock(void)
> {
> - unsigned char clk[STORE_CLOCK_EXT_SIZE];
> + char clk[STORE_CLOCK_EXT_SIZE];
>
> get_tod_clock_ext(clk);
> return *((unsigned long long *)&clk[1]);
> --
> 2.25.0
>

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200208140858.47970-1-natechancellor%40gmail.com.



--
Thanks,
~Nick Desaulniers

[Vasily Gorbik]

Applied, thanks.
I wonder though if Fixes: tag is really required for such changes. It
triggers stable backports (for all stable branches since v2.6.35) and
hence a lot of noise.

[Nick Desaulniers]

On Tue, Feb 11, 2020 at 5:12 AM Vasily Gorbik <g...@linux.ibm.com> wrote:
> Applied, thanks.

Hi Vasily, is this the expected tree+branch that the patch will be
pushed to? (I'm trying to track when+where our patches land).
https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/log/?h=fixes

--
Thanks,
~Nick Desaulniers

[Vasily Gorbik]

Hi Nick,
yes, this is correct. There might be some delay due to ci runs. But
this particular patch is now landed on that fixes branch.

[Nick Desaulniers]

Ah, yeah now I see it, thanks. The aggressive patch tracking helps us
figure out what landed where so that we can better backport patches to
LTS (hence the Fixes tags you've seen on other patches).
--
Thanks,
~Nick Desaulniers