All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] s390/time: Fix clk type in get_tod_clock
@ 2020-02-08 14:08 Nathan Chancellor
  2020-02-08 17:18 ` Nick Desaulniers
  2020-02-11 13:12 ` Vasily Gorbik
  0 siblings, 2 replies; 6+ messages in thread
From: Nathan Chancellor @ 2020-02-08 14:08 UTC (permalink / raw)
  To: Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-s390, linux-kernel, clang-built-linux, 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 <natechancellor@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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390/time: Fix clk type in get_tod_clock
  2020-02-08 14:08 [PATCH] s390/time: Fix clk type in get_tod_clock Nathan Chancellor
@ 2020-02-08 17:18 ` Nick Desaulniers
  2020-02-11 13:12 ` Vasily Gorbik
  1 sibling, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2020-02-08 17:18 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390,
	LKML, clang-built-linux

On Sat, Feb 8, 2020 at 3:10 PM Nathan Chancellor
<natechancellor@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 <natechancellor@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 <ndesaulniers@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-linux+unsubscribe@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390/time: Fix clk type in get_tod_clock
  2020-02-08 14:08 [PATCH] s390/time: Fix clk type in get_tod_clock Nathan Chancellor
  2020-02-08 17:18 ` Nick Desaulniers
@ 2020-02-11 13:12 ` Vasily Gorbik
  2020-02-11 17:30   ` Nick Desaulniers
  1 sibling, 1 reply; 6+ messages in thread
From: Vasily Gorbik @ 2020-02-11 13:12 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Heiko Carstens, Christian Borntraeger, linux-s390, linux-kernel,
	clang-built-linux

On Sat, Feb 08, 2020 at 07:08:59AM -0700, Nathan Chancellor 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 <natechancellor@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
> 
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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390/time: Fix clk type in get_tod_clock
  2020-02-11 13:12 ` Vasily Gorbik
@ 2020-02-11 17:30   ` Nick Desaulniers
  2020-02-11 19:53     ` Vasily Gorbik
  0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2020-02-11 17:30 UTC (permalink / raw)
  To: Vasily Gorbik
  Cc: Nathan Chancellor, Heiko Carstens, Christian Borntraeger,
	linux-s390, LKML, clang-built-linux

On Tue, Feb 11, 2020 at 5:12 AM Vasily Gorbik <gor@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390/time: Fix clk type in get_tod_clock
  2020-02-11 17:30   ` Nick Desaulniers
@ 2020-02-11 19:53     ` Vasily Gorbik
  2020-02-11 20:12       ` Nick Desaulniers
  0 siblings, 1 reply; 6+ messages in thread
From: Vasily Gorbik @ 2020-02-11 19:53 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Heiko Carstens, Christian Borntraeger,
	linux-s390, LKML, clang-built-linux

On Tue, Feb 11, 2020 at 05:30:24PM +0000, Nick Desaulniers wrote:
> On Tue, Feb 11, 2020 at 5:12 AM Vasily Gorbik <gor@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
> 
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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] s390/time: Fix clk type in get_tod_clock
  2020-02-11 19:53     ` Vasily Gorbik
@ 2020-02-11 20:12       ` Nick Desaulniers
  0 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2020-02-11 20:12 UTC (permalink / raw)
  To: Vasily Gorbik
  Cc: Nathan Chancellor, Heiko Carstens, Christian Borntraeger,
	linux-s390, LKML, clang-built-linux

On Tue, Feb 11, 2020 at 11:54 AM Vasily Gorbik <gor@linux.ibm.com> wrote:
>
> On Tue, Feb 11, 2020 at 05:30:24PM +0000, Nick Desaulniers wrote:
> > On Tue, Feb 11, 2020 at 5:12 AM Vasily Gorbik <gor@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
> >
> 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.
>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-11 20:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-08 14:08 [PATCH] s390/time: Fix clk type in get_tod_clock Nathan Chancellor
2020-02-08 17:18 ` Nick Desaulniers
2020-02-11 13:12 ` Vasily Gorbik
2020-02-11 17:30   ` Nick Desaulniers
2020-02-11 19:53     ` Vasily Gorbik
2020-02-11 20:12       ` Nick Desaulniers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.