Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1687185 > unrolled thread

[PATCH 00/14] gcc-7 warnings

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/14] gcc-7 warnings Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
    [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
      Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Joe Perches <joe@perches.com> - 2017-07-14 12:10 +0200
        Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 12:40 +0200
          Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Kevin Easton <kevin@guarana.org> - 2017-07-15 06:30 +0200
    [PATCH 04/14] x86: math-emu: avoid -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
    [PATCH 07/14] proc/kcore: hide a harmless warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
      Re: [PATCH 07/14] proc/kcore: hide a harmless warning Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-07-14 14:30 +0200
    [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning Joe Perches <joe@perches.com> - 2017-07-14 12:00 +0200
        Re: [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 12:30 +0200
    [PATCH 09/14] SFI: fix tautological-compare warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
    [PATCH 11/14] IB/uverbs: fix gcc-7 type warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 11/14] IB/uverbs: fix gcc-7 type warning Leon Romanovsky <leon@kernel.org> - 2017-07-14 11:50 +0200
    [PATCH 10/14] staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 10/14] staging:iio:resolver:ad2s1210 fix negative  IIO_ANGL_VEL read Jonathan Cameron <jic23@kernel.org> - 2017-07-15 13:50 +0200
    [PATCH 12/14] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
    [PATCH 06/14] acpi: thermal: fix gcc-6/ccache warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
    [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-14 21:30 +0200
        Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 22:20 +0200
          Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context  warning Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-07-14 23:50 +0200
    [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:50 +0200
      Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 14:10 +0200
        Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:30 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 15:00 +0200
            Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 15:20 +0200
              Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-14 21:40 +0200
      Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 14:50 +0200
      Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 15:50 +0200
        Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-17 16:30 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 16:40 +0200
            Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-17 23:30 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-17 16:40 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 16:40 +0200
    Re: [PATCH 00/14] gcc-7 warnings Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-14 12:30 +0200

Page 1 of 2  [1] 2  Next page →

#1687185 — [PATCH 00/14] gcc-7 warnings

This series should shut up all warnings introduced by gcc-6 or gcc-7 on
today's linux-next, as observed in "allmodconfig" builds on x86,
arm and arm64.

I have sent some of these before, but some others are new, as I had
at some point disabled the -Wint-in-bool-context warning in my
randconfig testing and did not notice the other warnings.

I have another series to address all -Wformat-overflow warnings,
and one more patch to turn off the -Wformat-truncation warnings
unless we build with "make W=1". I'll send that separately.

Most of these are consist of trivial refactoring of the code to
shut up false-positive warnings, the one exception being
"staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read",
which fixes a regression against linux-3.1 that has gone
unnoticed since then. Still, review from subsystem maintainers
would be appreciated.

I would suggest that Andrew Morton can pick these up into linux-mm
so we can make sure they all make it into the release. Alternatively
Linus might feel like picking them all up himself.

While I did not mark the harmless ones for stable backports,
Greg may also want to pick them up once they go upstream, to
help build-test the stable kernels with gcc-7.

      Arnd

Arnd Bergmann (14):
  [SUBMITTED 20170511] ide: avoid warning for timings calculation
  [SUBMITTED 20170511] ata: avoid gcc-7 warning in ata_timing_quantize
  [SUBMITTED 20170314] drm/vmwgfx: avoid gcc-7 parentheses warning
  x86: math-emu: avoid -Wint-in-bool-context warning
  isdn: isdnloop: suppress a gcc-7 warning
  acpi: thermal: fix gcc-6/ccache warning
  proc/kcore: hide a harmless warning
  Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning
  SFI: fix tautological-compare warning
  staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read
  IB/uverbs: fix gcc-7 type warning
  drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
  iopoll: avoid -Wint-in-bool-context warning
  [media] fix warning on v4l2_subdev_call() result interpreted as bool

 arch/x86/math-emu/fpu_emu.h                          |  2 +-
 drivers/acpi/processor_thermal.c                     |  6 ++++--
 drivers/ata/libata-core.c                            | 20 ++++++++++----------
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c      |  6 +++---
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c              |  2 +-
 drivers/ide/ide-timings.c                            | 18 +++++++++---------
 drivers/infiniband/core/uverbs.h                     | 14 ++++++++------
 drivers/input/misc/adxl34x.c                         |  2 +-
 drivers/isdn/isdnloop/isdnloop.c                     |  2 +-
 drivers/media/pci/cx18/cx18-ioctl.c                  |  6 ++++--
 drivers/media/pci/saa7146/mxb.c                      |  5 +++--
 drivers/media/platform/atmel/atmel-isc.c             |  4 ++--
 drivers/media/platform/atmel/atmel-isi.c             |  4 ++--
 drivers/media/platform/blackfin/bfin_capture.c       |  4 ++--
 drivers/media/platform/omap3isp/ispccdc.c            |  5 +++--
 drivers/media/platform/pxa_camera.c                  |  3 ++-
 drivers/media/platform/rcar-vin/rcar-core.c          |  2 +-
 drivers/media/platform/rcar-vin/rcar-dma.c           |  4 +++-
 drivers/media/platform/soc_camera/soc_camera.c       |  4 ++--
 drivers/media/platform/stm32/stm32-dcmi.c            |  4 ++--
 drivers/media/platform/ti-vpe/cal.c                  |  6 ++++--
 drivers/sfi/sfi_core.c                               |  9 ++++++---
 drivers/staging/iio/resolver/ad2s1210.c              |  2 +-
 .../staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 13 +++++++------
 fs/proc/kcore.c                                      | 10 ++++++----
 include/linux/iopoll.h                               |  6 ++++--
 include/linux/regmap.h                               |  2 +-
 27 files changed, 93 insertions(+), 72 deletions(-)

-- 
2.9.0

[toc] | [next] | [standalone]

#1687186 — [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning

We test whether a bit is set in a mask here, which is correct
but gcc warns about it as it thinks it might be confusing:

drivers/isdn/isdnloop/isdnloop.c:412:37: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context]

This replaces the negation of an integer with an equivalent
comparison to zero, which gets rid of the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/isdn/isdnloop/isdnloop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index 6ffd13466b8c..5792928b944d 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -409,7 +409,7 @@ isdnloop_sendbuf(int channel, struct sk_buff *skb, isdnloop_card *card)
 		return -EINVAL;
 	}
 	if (len) {
-		if (!(card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE))
+		if ((card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE) == 0)
 			return 0;
 		if (card->sndcount[channel] > ISDNLOOP_MAX_SQUEUE)
 			return 0;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687227 — Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning

On Fri, 2017-07-14 at 11:25 +0200, Arnd Bergmann wrote:
> We test whether a bit is set in a mask here, which is correct
> but gcc warns about it as it thinks it might be confusing:
> 
> drivers/isdn/isdnloop/isdnloop.c:412:37: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context]
> 
> This replaces the negation of an integer with an equivalent
> comparison to zero, which gets rid of the warning.
[]
> diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
[]
> @@ -409,7 +409,7 @@ isdnloop_sendbuf(int channel, struct sk_buff *skb, isdnloop_card *card)
>  		return -EINVAL;
>  	}
>  	if (len) {
> -		if (!(card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE))
> +		if ((card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE) == 0)
>  			return 0;
>  		if (card->sndcount[channel] > ISDNLOOP_MAX_SQUEUE)
>  			return 0;

The if as written can not be zero.

drivers/isdn/isdnloop/isdnloop.h:#define ISDNLOOP_FLAGS_B1ACTIVE 1      /* B-Channel-1 is open           */
drivers/isdn/isdnloop/isdnloop.h:#define ISDNLOOP_FLAGS_B2ACTIVE 2      /* B-Channel-2 is open           */

Perhaps this is a logic defect and should be:

		if (!(card->flags & ((channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE)))

[toc] | [prev] | [next] | [standalone]

#1687240 — Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning

On Fri, Jul 14, 2017 at 12:08 PM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2017-07-14 at 11:25 +0200, Arnd Bergmann wrote:
>> We test whether a bit is set in a mask here, which is correct
>> but gcc warns about it as it thinks it might be confusing:
>>
>> drivers/isdn/isdnloop/isdnloop.c:412:37: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context]
>>
>> This replaces the negation of an integer with an equivalent
>> comparison to zero, which gets rid of the warning.
> []
>> diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
> []
>> @@ -409,7 +409,7 @@ isdnloop_sendbuf(int channel, struct sk_buff *skb, isdnloop_card *card)
>>               return -EINVAL;
>>       }
>>       if (len) {
>> -             if (!(card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE))
>> +             if ((card->flags & (channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE) == 0)
>>                       return 0;
>>               if (card->sndcount[channel] > ISDNLOOP_MAX_SQUEUE)
>>                       return 0;
>
> The if as written can not be zero.
>
> drivers/isdn/isdnloop/isdnloop.h:#define ISDNLOOP_FLAGS_B1ACTIVE 1      /* B-Channel-1 is open           */
> drivers/isdn/isdnloop/isdnloop.h:#define ISDNLOOP_FLAGS_B2ACTIVE 2      /* B-Channel-2 is open           */
>
> Perhaps this is a logic defect and should be:
>
>                 if (!(card->flags & ((channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE)))

Yes, good catch. I had thought about it for a bit whether that would be
the answer, but come to the wrong conclusion on my own.

Note that the version you suggested will still have the warning, so I think
it needs to be

                 if (card->flags &
                    ((channel) ? ISDNLOOP_FLAGS_B2ACTIVE :
ISDNLOOP_FLAGS_B1ACTIVE)
                     == 0)

or something like that, probably having a temporary flag variable would be best:

          int flag =  channel ? ISDNLOOP_FLAGS_B2ACTIVE :
ISDNLOOP_FLAGS_B1ACTIVE;
          if ((card->flags & flag) == 0)
                       return 0;

         Arnd

[toc] | [prev] | [next] | [standalone]

#1687854 — Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning

On Fri, Jul 14, 2017 at 12:37:05PM +0200, Arnd Bergmann wrote:
> On Fri, Jul 14, 2017 at 12:08 PM, Joe Perches <joe@perches.com> wrote:
> > On Fri, 2017-07-14 at 11:25 +0200, Arnd Bergmann wrote:
> >> We test whether a bit is set in a mask here, which is correct
> >> but gcc warns about it as it thinks it might be confusing:
> >>
> >> drivers/isdn/isdnloop/isdnloop.c:412:37: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context]

...

> > Perhaps this is a logic defect and should be:
> >
> >                 if (!(card->flags & ((channel) ? ISDNLOOP_FLAGS_B2ACTIVE : ISDNLOOP_FLAGS_B1ACTIVE)))
> 
> Yes, good catch. I had thought about it for a bit whether that would be
> the answer, but come to the wrong conclusion on my own.
> 
> Note that the version you suggested will still have the warning, so I think
> it needs to be

It shouldn't - the warning is for using an integer *constant* in boolean
context, but the result of & isn't a constant and should be fine.

!(flags & mask) is a very common idiom.

    - Kevin 

[toc] | [prev] | [next] | [standalone]

#1687187 — [PATCH 04/14] x86: math-emu: avoid -Wint-in-bool-context warning

The setsign() macro gets called with an integer argument in a
few places, leading to a harmless warning in gcc-7:

arch/x86/math-emu/reg_add_sub.c: In function 'FPU_add':
arch/x86/math-emu/reg_add_sub.c:80:48: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context]

This turns the integer into a boolean expression by comparing it
to zero.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/math-emu/fpu_emu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/math-emu/fpu_emu.h b/arch/x86/math-emu/fpu_emu.h
index afbc4d805d66..c9c320dccca1 100644
--- a/arch/x86/math-emu/fpu_emu.h
+++ b/arch/x86/math-emu/fpu_emu.h
@@ -157,7 +157,7 @@ extern u_char const data_sizes_16[32];
 
 #define signbyte(a) (((u_char *)(a))[9])
 #define getsign(a) (signbyte(a) & 0x80)
-#define setsign(a,b) { if (b) signbyte(a) |= 0x80; else signbyte(a) &= 0x7f; }
+#define setsign(a,b) { if ((b) != 0) signbyte(a) |= 0x80; else signbyte(a) &= 0x7f; }
 #define copysign(a,b) { if (getsign(a)) signbyte(b) |= 0x80; \
                         else signbyte(b) &= 0x7f; }
 #define changesign(a) { signbyte(a) ^= 0x80; }
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687189 — [PATCH 07/14] proc/kcore: hide a harmless warning

gcc warns when MODULES_VADDR/END is defined to the same value as
VMALLOC_START/VMALLOC_END, e.g. on x86-32:

fs/proc/kcore.c: In function ‘add_modules_range’:
fs/proc/kcore.c:622:161: error: self-comparison always evaluates to false [-Werror=tautological-compare]
  if (/*MODULES_VADDR != VMALLOC_START && */MODULES_END != VMALLOC_END) {

The code is correct as it is required for most other configurations.
The best workaround I found for shutting up that warning is to make
it a little more complex by adding a temporary variable. The compiler
will still optimize away the code as it finds the two to be identical,
but it no longer warns because it doesn't condider the comparison
"tautological" any more.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/proc/kcore.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index 45629f4b5402..c503ad657c46 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -620,12 +620,14 @@ static void __init proc_kcore_text_init(void)
 /*
  * MODULES_VADDR has no intersection with VMALLOC_ADDR.
  */
-struct kcore_list kcore_modules;
+static struct kcore_list kcore_modules;
 static void __init add_modules_range(void)
 {
-	if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
-		kclist_add(&kcore_modules, (void *)MODULES_VADDR,
-			MODULES_END - MODULES_VADDR, KCORE_VMALLOC);
+	void *start = (void *)MODULES_VADDR;
+	size_t len = MODULES_END - MODULES_VADDR;
+
+	if (start != (void *)VMALLOC_START && len != VMALLOC_END - VMALLOC_START) {
+		kclist_add(&kcore_modules, start, len, KCORE_VMALLOC);
 	}
 }
 #else
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687335 — Re: [PATCH 07/14] proc/kcore: hide a harmless warning

On 14 July 2017 at 10:25, Arnd Bergmann <arnd@arndb.de> wrote:
> gcc warns when MODULES_VADDR/END is defined to the same value as
> VMALLOC_START/VMALLOC_END, e.g. on x86-32:
>
> fs/proc/kcore.c: In function ‘add_modules_range’:
> fs/proc/kcore.c:622:161: error: self-comparison always evaluates to false [-Werror=tautological-compare]
>   if (/*MODULES_VADDR != VMALLOC_START && */MODULES_END != VMALLOC_END) {
>

Does it occur for subtraction as well? Or only for comparison?

> The code is correct as it is required for most other configurations.
> The best workaround I found for shutting up that warning is to make
> it a little more complex by adding a temporary variable. The compiler
> will still optimize away the code as it finds the two to be identical,
> but it no longer warns because it doesn't condider the comparison
> "tautological" any more.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/proc/kcore.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
> index 45629f4b5402..c503ad657c46 100644
> --- a/fs/proc/kcore.c
> +++ b/fs/proc/kcore.c
> @@ -620,12 +620,14 @@ static void __init proc_kcore_text_init(void)
>  /*
>   * MODULES_VADDR has no intersection with VMALLOC_ADDR.
>   */
> -struct kcore_list kcore_modules;
> +static struct kcore_list kcore_modules;
>  static void __init add_modules_range(void)
>  {
> -       if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
> -               kclist_add(&kcore_modules, (void *)MODULES_VADDR,
> -                       MODULES_END - MODULES_VADDR, KCORE_VMALLOC);
> +       void *start = (void *)MODULES_VADDR;
> +       size_t len = MODULES_END - MODULES_VADDR;
> +
> +       if (start != (void *)VMALLOC_START && len != VMALLOC_END - VMALLOC_START) {
> +               kclist_add(&kcore_modules, start, len, KCORE_VMALLOC);
>         }
>  }
>  #else
> --
> 2.9.0
>

[toc] | [prev] | [next] | [standalone]

#1687192 — [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning

When we pass the result of a multiplication as the timeout, we
can get a warning:

drivers/mmc/host/bcm2835.c:596:149: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
drivers/mfd/arizona-core.c:247:195: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]

This is easy to avoid by comparing the timeout to zero instead,
making it a boolean expression.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/iopoll.h | 6 ++++--
 include/linux/regmap.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
index d29e1e21bf3f..7a17ba02253b 100644
--- a/include/linux/iopoll.h
+++ b/include/linux/iopoll.h
@@ -48,7 +48,8 @@
 		(val) = op(addr); \
 		if (cond) \
 			break; \
-		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+		if ((timeout_us) > 0 && \
+		    ktime_compare(ktime_get(), timeout) > 0) { \
 			(val) = op(addr); \
 			break; \
 		} \
@@ -82,7 +83,8 @@
 		(val) = op(addr); \
 		if (cond) \
 			break; \
-		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+		if ((timeout_us) > 0 && \
+		    ktime_compare(ktime_get(), timeout) > 0) { \
 			(val) = op(addr); \
 			break; \
 		} \
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 1474ab0a3922..0889dbf37161 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -129,7 +129,7 @@ struct reg_sequence {
 			break; \
 		if (cond) \
 			break; \
-		if ((timeout_us) && \
+		if ((timeout_us) > 0 && \
 		    ktime_compare(ktime_get(), __timeout) > 0) { \
 			__ret = regmap_read((map), (addr), &(val)); \
 			break; \
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687218 — Re: [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning

On Fri, 2017-07-14 at 11:31 +0200, Arnd Bergmann wrote:
> When we pass the result of a multiplication as the timeout, we
> can get a warning:
> 
> drivers/mmc/host/bcm2835.c:596:149: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
> drivers/mfd/arizona-core.c:247:195: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
> 
> This is easy to avoid by comparing the timeout to zero instead,
> making it a boolean expression.

Perhaps this is better as != 0 if the multiply is signed.

> diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
[]
> @@ -48,7 +48,8 @@
>  		(val) = op(addr); \
>  		if (cond) \
>  			break; \
> -		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
> +		if ((timeout_us) > 0 && \
> +		    ktime_compare(ktime_get(), timeout) > 0) { \
>  			(val) = op(addr); \
>  			break; \
>  		} \

etc...

[toc] | [prev] | [next] | [standalone]

#1687236 — Re: [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning

On Fri, Jul 14, 2017 at 11:55 AM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2017-07-14 at 11:31 +0200, Arnd Bergmann wrote:
>> When we pass the result of a multiplication as the timeout, we
>> can get a warning:
>>
>> drivers/mmc/host/bcm2835.c:596:149: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
>> drivers/mfd/arizona-core.c:247:195: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
>>
>> This is easy to avoid by comparing the timeout to zero instead,
>> making it a boolean expression.
>
> Perhaps this is better as != 0 if the multiply is signed.

I thought about that, but decided that as a negative timeout_us already
gives us rather random behavior (ktime_add_us() takes an unsigned
argument), the '>' comparison gives a more well-defined result by
ignoring the timeout.

         Arnd

[toc] | [prev] | [next] | [standalone]

#1687194 — [PATCH 09/14] SFI: fix tautological-compare warning

With ccache in combination with gcc-6, we get a harmless warning for the sfi subsystem,
as ccache only sees the preprocessed source:

drivers/sfi/sfi_core.c: In function ‘sfi_map_table’:
drivers/sfi/sfi_core.c:175:53: error: self-comparison always evaluates to true [-Werror=tautological-compare]

Using an inline function to do the comparison tells the compiler what is
going on even for preprocessed files, and avoids the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/sfi/sfi_core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
index 296db7a69c27..a8f2313a2613 100644
--- a/drivers/sfi/sfi_core.c
+++ b/drivers/sfi/sfi_core.c
@@ -71,9 +71,12 @@
 
 #include "sfi_core.h"
 
-#define ON_SAME_PAGE(addr1, addr2) \
-	(((unsigned long)(addr1) & PAGE_MASK) == \
-	((unsigned long)(addr2) & PAGE_MASK))
+static inline bool on_same_page(unsigned long addr1, unsigned long addr2)
+{
+	return (addr1 & PAGE_MASK) == (addr2 & PAGE_MASK);
+}
+
+#define ON_SAME_PAGE(addr1, addr2) on_same_page((unsigned long)addr1, (unsigned long)addr2)
 #define TABLE_ON_PAGE(page, table, size) (ON_SAME_PAGE(page, table) && \
 				ON_SAME_PAGE(page, table + size))
 
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687196 — [PATCH 11/14] IB/uverbs: fix gcc-7 type warning

When using ccache, we get a harmless warning about the fact that
we use the result of a multiplication as a condition:

drivers/infiniband/core/uverbs_main.c: In function 'ib_uverbs_write':
drivers/infiniband/core/uverbs_main.c:787:40: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
drivers/infiniband/core/uverbs_main.c:787:117: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
drivers/infiniband/core/uverbs_main.c:790:50: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
drivers/infiniband/core/uverbs_main.c:790:151: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]

This changes the macro to explicitly check the number for a positive
length, which avoids the warning.

Fixes: a96e4e2ffe43 ("IB/uverbs: New macro to set pointers to NULL if length is 0 in INIT_UDATA()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/infiniband/core/uverbs.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index 64d494a64daf..364d7de05721 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -55,12 +55,14 @@
 		(udata)->outlen = (olen);				\
 	} while (0)
 
-#define INIT_UDATA_BUF_OR_NULL(udata, ibuf, obuf, ilen, olen)			\
-	do {									\
-		(udata)->inbuf  = (ilen) ? (const void __user *) (ibuf) : NULL;	\
-		(udata)->outbuf = (olen) ? (void __user *) (obuf) : NULL;	\
-		(udata)->inlen  = (ilen);					\
-		(udata)->outlen = (olen);					\
+#define INIT_UDATA_BUF_OR_NULL(udata, ibuf, obuf, ilen, olen)		\
+	do {								\
+		(udata)->inbuf  = (ilen) > 0 ?				\
+				  (const void __user *) (ibuf) : NULL;	\
+		(udata)->outbuf = (olen) > 0 ?				\
+				  (void __user *) (obuf) : NULL;	\
+		(udata)->inlen  = (ilen);				\
+		(udata)->outlen = (olen);				\
 	} while (0)
 
 /*
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687206 — Re: [PATCH 11/14] IB/uverbs: fix gcc-7 type warning

[Multipart message — attachments visible in raw view] — view raw

On Fri, Jul 14, 2017 at 11:31:04AM +0200, Arnd Bergmann wrote:
> When using ccache, we get a harmless warning about the fact that
> we use the result of a multiplication as a condition:
>
> drivers/infiniband/core/uverbs_main.c: In function 'ib_uverbs_write':
> drivers/infiniband/core/uverbs_main.c:787:40: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
> drivers/infiniband/core/uverbs_main.c:787:117: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
> drivers/infiniband/core/uverbs_main.c:790:50: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
> drivers/infiniband/core/uverbs_main.c:790:151: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
>
> This changes the macro to explicitly check the number for a positive
> length, which avoids the warning.
>
> Fixes: a96e4e2ffe43 ("IB/uverbs: New macro to set pointers to NULL if length is 0 in INIT_UDATA()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/infiniband/core/uverbs.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

[toc] | [prev] | [next] | [standalone]

#1687197 — [PATCH 10/14] staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read

gcc-7 points out an older regression:

drivers/staging/iio/resolver/ad2s1210.c: In function 'ad2s1210_read_raw':
drivers/staging/iio/resolver/ad2s1210.c:515:42: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context]

The original code had 'unsigned short' here, but incorrectly got
converted to 'bool'. This reverts the regression and uses a normal
type instead.

Fixes: 29148543c521 ("staging:iio:resolver:ad2s1210 minimal chan spec conversion.")
Cc: stable@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/iio/resolver/ad2s1210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index a6a8393d6664..3e00df74b18c 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -472,7 +472,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
 			     long m)
 {
 	struct ad2s1210_state *st = iio_priv(indio_dev);
-	bool negative;
+	u16 negative;
 	int ret = 0;
 	u16 pos;
 	s16 vel;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687924 — Re: [PATCH 10/14] staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read

On Fri, 14 Jul 2017 11:31:03 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> gcc-7 points out an older regression:
> 
> drivers/staging/iio/resolver/ad2s1210.c: In function 'ad2s1210_read_raw':
> drivers/staging/iio/resolver/ad2s1210.c:515:42: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context]
> 
> The original code had 'unsigned short' here, but incorrectly got
> converted to 'bool'. This reverts the regression and uses a normal
> type instead.
> 
> Fixes: 29148543c521 ("staging:iio:resolver:ad2s1210 minimal chan spec conversion.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks Arnd,

Applied to the fixes-togreg branch of iio.git.

Jonathan
> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index a6a8393d6664..3e00df74b18c 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -472,7 +472,7 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
>  			     long m)
>  {
>  	struct ad2s1210_state *st = iio_priv(indio_dev);
> -	bool negative;
> +	u16 negative;
>  	int ret = 0;
>  	u16 pos;
>  	s16 vel;

[toc] | [prev] | [next] | [standalone]

#1687198 — [PATCH 12/14] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning

gcc thinks that interpreting a multiplication result as a bool
is confusing:

drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c: In function 'read_pll':
drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c:133:8: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]

In this instance, I think using multiplication is more intuitive
than '&&', so I'm adding a comparison to zero instead to shut up
the warning. To further improve readability, I also make the
error case indented and leave the normal case as the final 'return'
statement.

Fixes: 7632b30e4b8b ("drm/nouveau/clk: namespace + nvidia gpu names (no binary change)")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
index 96e0941c8edd..04b4f4ccf186 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c
@@ -130,10 +130,10 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll)
 		sclk = read_clk(clk, 0x10 + idx, false);
 	}
 
-	if (M * P)
-		return sclk * N / (M * P);
+	if (M * P == 0)
+		return 0;
 
-	return 0;
+	return sclk * N / (M * P);
 }
 
 static int
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687199 — [PATCH 06/14] acpi: thermal: fix gcc-6/ccache warning

In some configurations, topology_physical_package_id() is trivially
defined as '-1' for any input, resulting a comparison that is
always true:

drivers/acpi/processor_thermal.c: In function ‘cpufreq_set_cur_state’:
drivers/acpi/processor_thermal.c:137:36: error: self-comparison always evaluates to true [-Werror=tautological-compare]

By introducing a temporary variable, we can tell gcc that this is
intentional.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/acpi/processor_thermal.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 59c3a5d1e600..411f3a7f4a7c 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -122,20 +122,22 @@ static int cpufreq_get_cur_state(unsigned int cpu)
 static int cpufreq_set_cur_state(unsigned int cpu, int state)
 {
 	int i;
+	int id;
 
 	if (!cpu_has_cpufreq(cpu))
 		return 0;
 
 	reduction_pctg(cpu) = state;
 
+	id = topology_physical_package_id(cpu);
+
 	/*
 	 * Update all the CPUs in the same package because they all
 	 * contribute to the temperature and often share the same
 	 * frequency.
 	 */
 	for_each_online_cpu(i) {
-		if (topology_physical_package_id(i) ==
-		    topology_physical_package_id(cpu))
+		if (topology_physical_package_id(i) == id)
 			cpufreq_update_policy(i);
 	}
 	return 0;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687202 — [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning

FIFO_MODE is an macro expression with a '<<' operator, which
gcc points out could be misread as a '<':

drivers/input/misc/adxl34x.c: In function 'adxl34x_probe':
drivers/input/misc/adxl34x.c:799:36: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context]

This converts the test to an explicit comparison with zero,
making it clearer to gcc and the reader what is intended.

Fixes: e27c729219ad ("Input: add driver for ADXL345/346 Digital Accelerometers")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/input/misc/adxl34x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
index 2b2d02f408bb..e0caaa0de454 100644
--- a/drivers/input/misc/adxl34x.c
+++ b/drivers/input/misc/adxl34x.c
@@ -796,7 +796,7 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 
 	if (pdata->watermark) {
 		ac->int_mask |= WATERMARK;
-		if (!FIFO_MODE(pdata->fifo_mode))
+		if (FIFO_MODE(pdata->fifo_mode) == 0)
 			ac->pdata.fifo_mode |= FIFO_STREAM;
 	} else {
 		ac->int_mask |= DATA_READY;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687613 — Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning

On Fri, Jul 14, 2017 at 2:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> FIFO_MODE is an macro expression with a '<<' operator, which
> gcc points out could be misread as a '<':

Yeah, no, NAK again.

We don't make the code look worse just because gcc is being a f*cking
moron about things.

This warning is clearly pure garbage.

                 Linus

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web