Lots of new warnings with gcc-7.1.1

Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]

So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.=
1.1

Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.

Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.

At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.

After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.

But a lot of the warnings look reasonable, and at least the warnings
are nice in how they actually explain why the warning is happening.
Example:

  arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In
function =E2=80=98max7315_platform_data=E2=80=99:
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:35:
warning: =E2=80=98%d=E2=80=99 directive writing between 1 and 11 bytes into=
 a region
of size 9 [-Wformat-overflow=3D]
     sprintf(base_pin_name, "max7315_%d_base", nr);
                                     ^~
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26:
note: directive argument in the range [-2147483647, 2147483647]

Yeah, the compiler is technically correct, but we already made sure we
have at most MAX7315_NUM of those adapters, so no, "nr" is really not
going to be a 10-digit number.

So the warning is kind of bogus.

At the same time, others aren't quite as insane, and in many cases the
warnings might be easy to just fix.

And some actually look valid, although they might still require odd input:

  net/bluetooth/smp.c: In function =E2=80=98le_max_key_size_read=E2=80=99:
  net/bluetooth/smp.c:3372:29: warning: =E2=80=98snprintf=E2=80=99 output m=
ay be
truncated before the last format character [-Wformat-truncation=3D]
    snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
                               ^~~~~~~
  net/bluetooth/smp.c:3372:2: note: =E2=80=98snprintf=E2=80=99 output betwe=
en 4 and 5
bytes into a destination of size 4

yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
really does need 5 bytes for "%2u\n" with the terminating NUL
character.

Of course, the "%2d" implies that people expect it to be < 100, but at
the same time it doesn't sound like a bad idea to just make the buffer
be one byte bigger. So..

Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.

I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=3D1" kind of warning.  But let's see how
many of these we can fix, ok?

                  Linus

Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]

So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1

Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.

Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.

At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.

After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.

But a lot of the warnings look reasonable, and at least the warnings
are nice in how they actually explain why the warning is happening.
Example:

  arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In
function ‘max7315_platform_data’:
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:35:
warning: ‘%d’ directive writing between 1 and 11 bytes into a region
of size 9 [-Wformat-overflow=]
     sprintf(base_pin_name, "max7315_%d_base", nr);
                                     ^~
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26:
note: directive argument in the range [-2147483647, 2147483647]

Yeah, the compiler is technically correct, but we already made sure we
have at most MAX7315_NUM of those adapters, so no, "nr" is really not
going to be a 10-digit number.

So the warning is kind of bogus.

At the same time, others aren't quite as insane, and in many cases the
warnings might be easy to just fix.

And some actually look valid, although they might still require odd input:

  net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
  net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
truncated before the last format character [-Wformat-truncation=]
    snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
                               ^~~~~~~
  net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
bytes into a destination of size 4

yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
really does need 5 bytes for "%2u\n" with the terminating NUL
character.

Of course, the "%2d" implies that people expect it to be < 100, but at
the same time it doesn't sound like a bad idea to just make the buffer
be one byte bigger. So..

Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.

I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
many of these we can fix, ok?

                  Linus

Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]

So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1

Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.

Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.

At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.

After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.

But a lot of the warnings look reasonable, and at least the warnings
are nice in how they actually explain why the warning is happening.
Example:

  arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In
function ‘max7315_platform_data’:
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:35:
warning: ‘%d’ directive writing between 1 and 11 bytes into a region
of size 9 [-Wformat-overflow=]
     sprintf(base_pin_name, "max7315_%d_base", nr);
                                     ^~
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26:
note: directive argument in the range [-2147483647, 2147483647]

Yeah, the compiler is technically correct, but we already made sure we
have at most MAX7315_NUM of those adapters, so no, "nr" is really not
going to be a 10-digit number.

So the warning is kind of bogus.

At the same time, others aren't quite as insane, and in many cases the
warnings might be easy to just fix.

And some actually look valid, although they might still require odd input:

  net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
  net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
truncated before the last format character [-Wformat-truncation=]
    snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
                               ^~~~~~~
  net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
bytes into a destination of size 4

yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
really does need 5 bytes for "%2u\n" with the terminating NUL
character.

Of course, the "%2d" implies that people expect it to be < 100, but at
the same time it doesn't sound like a bad idea to just make the buffer
be one byte bigger. So..

Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.

I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
many of these we can fix, ok?

                  Linus

Re: Lots of new warnings with gcc-7.1.1

[Marcel Holtmann]

Hi Linus,

> At the same time, others aren't quite as insane, and in many cases the
> warnings might be easy to just fix.
> 
> And some actually look valid, although they might still require odd input:
> 
>  net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
>  net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
> truncated before the last format character [-Wformat-truncation=]
>    snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
>                               ^~~~~~~
>  net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
> bytes into a destination of size 4
> 
> yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
> really does need 5 bytes for "%2u\n" with the terminating NUL
> character.
> 
> Of course, the "%2d" implies that people expect it to be < 100, but at
> the same time it doesn't sound like a bad idea to just make the buffer
> be one byte bigger. So..

the Bluetooth specification defines that the Maximum Encryption Key Size shall be in the range 7 to 16 octets. Which is also reflected in these defines:

#define SMP_MIN_ENC_KEY_SIZE            7
#define SMP_MAX_ENC_KEY_SIZE            16

So it is buf[4] since we know it never gets larger than 16. So even in this case the warning is bogus.

I have no problem in increasing it to buf[5] to shut up the compiler, but that is what I would be doing here. I am not fixing an actual bug.

> Anyway, it would be lovely if some of the more affected developers
> would take a look at gcc-7.1.1 warnings. Right now I get about three
> *thousand* lines of warnings from a "make allmodconfig" build, which
> makes them a bit overwhelming.
> 
> I do suspect I'll make "-Wformat-truncation" (as opposed to
> "-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
> many of these we can fix, ok?

I had to use the -Wno-format-trunction in a few projects since gcc was completely lost. And since we were using snprintf, I saw no point in trying to please gcc with a larger buffer.

Regards

Marcel

Re: Lots of new warnings with gcc-7.1.1

[Marcel Holtmann]

Hi Linus,

> At the same time, others aren't quite as insane, and in many cases the
> warnings might be easy to just fix.
> 
> And some actually look valid, although they might still require odd input:
> 
>  net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
>  net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
> truncated before the last format character [-Wformat-truncation=]
>    snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
>                               ^~~~~~~
>  net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
> bytes into a destination of size 4
> 
> yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
> really does need 5 bytes for "%2u\n" with the terminating NUL
> character.
> 
> Of course, the "%2d" implies that people expect it to be < 100, but at
> the same time it doesn't sound like a bad idea to just make the buffer
> be one byte bigger. So..

the Bluetooth specification defines that the Maximum Encryption Key Size shall be in the range 7 to 16 octets. Which is also reflected in these defines:

#define SMP_MIN_ENC_KEY_SIZE            7
#define SMP_MAX_ENC_KEY_SIZE            16

So it is buf[4] since we know it never gets larger than 16. So even in this case the warning is bogus.

I have no problem in increasing it to buf[5] to shut up the compiler, but that is what I would be doing here. I am not fixing an actual bug.

> Anyway, it would be lovely if some of the more affected developers
> would take a look at gcc-7.1.1 warnings. Right now I get about three
> *thousand* lines of warnings from a "make allmodconfig" build, which
> makes them a bit overwhelming.
> 
> I do suspect I'll make "-Wformat-truncation" (as opposed to
> "-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
> many of these we can fix, ok?

I had to use the -Wno-format-trunction in a few projects since gcc was completely lost. And since we were using snprintf, I saw no point in trying to please gcc with a larger buffer.

Regards

Marcel

Re: Lots of new warnings with gcc-7.1.1

[Marcel Holtmann]

Hi Linus,

> At the same time, others aren't quite as insane, and in many cases the
> warnings might be easy to just fix.
> 
> And some actually look valid, although they might still require odd input:
> 
>  net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
>  net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
> truncated before the last format character [-Wformat-truncation=]
>    snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
>                               ^~~~~~~
>  net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
> bytes into a destination of size 4
> 
> yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
> really does need 5 bytes for "%2u\n" with the terminating NUL
> character.
> 
> Of course, the "%2d" implies that people expect it to be < 100, but at
> the same time it doesn't sound like a bad idea to just make the buffer
> be one byte bigger. So..

the Bluetooth specification defines that the Maximum Encryption Key Size shall be in the range 7 to 16 octets. Which is also reflected in these defines:

#define SMP_MIN_ENC_KEY_SIZE            7
#define SMP_MAX_ENC_KEY_SIZE            16

So it is buf[4] since we know it never gets larger than 16. So even in this case the warning is bogus.

I have no problem in increasing it to buf[5] to shut up the compiler, but that is what I would be doing here. I am not fixing an actual bug.

> Anyway, it would be lovely if some of the more affected developers
> would take a look at gcc-7.1.1 warnings. Right now I get about three
> *thousand* lines of warnings from a "make allmodconfig" build, which
> makes them a bit overwhelming.
> 
> I do suspect I'll make "-Wformat-truncation" (as opposed to
> "-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
> many of these we can fix, ok?

I had to use the -Wno-format-trunction in a few projects since gcc was completely lost. And since we were using snprintf, I saw no point in trying to please gcc with a larger buffer.

Regards

Marcel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Guenter Roeck]

On 07/11/2017 03:35 PM, Linus Torvalds wrote:
> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]
> 
> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
> 
> Which in turn means that my nice clean allmodconfig compile is not an
> unholy mess of annoying new warnings.
> 
> Normally I hate the stupid new warnings, but this time around they are
> actually exactly the kinds of warnings you'd want to see and that are
> hard for humans to pick out errors: lots of format errors wrt limited
> buffer sizes.
> 
> At the same time, many of them *are* annoying. We have various limited
> buffers that are limited for a good reason, and some of the format
> truncation warnings are about numbers in the range {0-MAX_INT], where
> we definitely know that we don't need to worry about the really big
> ones.
> 
> After all, we're using "snprintf()" for a reason - we *want* to
> truncate if the buffer is too small.
> 

The hwmon warnings are all about supporting no more than 9,999 sensors
(applesmc) to 999,999,999 sensors (scpi) of a given type. Easy "fix" would
be to replace snprintf() with scnprintf(), presumably because gcc doesn't
know about scnprintf(). We could also increase the name buffer size.
But is that really worth it just to silence gcc ?

Guenter

Re: Lots of new warnings with gcc-7.1.1

[Guenter Roeck]

On 07/11/2017 03:35 PM, Linus Torvalds wrote:
> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]
> 
> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
> 
> Which in turn means that my nice clean allmodconfig compile is not an
> unholy mess of annoying new warnings.
> 
> Normally I hate the stupid new warnings, but this time around they are
> actually exactly the kinds of warnings you'd want to see and that are
> hard for humans to pick out errors: lots of format errors wrt limited
> buffer sizes.
> 
> At the same time, many of them *are* annoying. We have various limited
> buffers that are limited for a good reason, and some of the format
> truncation warnings are about numbers in the range {0-MAX_INT], where
> we definitely know that we don't need to worry about the really big
> ones.
> 
> After all, we're using "snprintf()" for a reason - we *want* to
> truncate if the buffer is too small.
> 

The hwmon warnings are all about supporting no more than 9,999 sensors
(applesmc) to 999,999,999 sensors (scpi) of a given type. Easy "fix" would
be to replace snprintf() with scnprintf(), presumably because gcc doesn't
know about scnprintf(). We could also increase the name buffer size.
But is that really worth it just to silence gcc ?

Guenter

Re: Lots of new warnings with gcc-7.1.1

[Guenter Roeck]

On 07/11/2017 03:35 PM, Linus Torvalds wrote:
> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]
> 
> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
> 
> Which in turn means that my nice clean allmodconfig compile is not an
> unholy mess of annoying new warnings.
> 
> Normally I hate the stupid new warnings, but this time around they are
> actually exactly the kinds of warnings you'd want to see and that are
> hard for humans to pick out errors: lots of format errors wrt limited
> buffer sizes.
> 
> At the same time, many of them *are* annoying. We have various limited
> buffers that are limited for a good reason, and some of the format
> truncation warnings are about numbers in the range {0-MAX_INT], where
> we definitely know that we don't need to worry about the really big
> ones.
> 
> After all, we're using "snprintf()" for a reason - we *want* to
> truncate if the buffer is too small.
> 

The hwmon warnings are all about supporting no more than 9,999 sensors
(applesmc) to 999,999,999 sensors (scpi) of a given type. Easy "fix" would
be to replace snprintf() with scnprintf(), presumably because gcc doesn't
know about scnprintf(). We could also increase the name buffer size.
But is that really worth it just to silence gcc ?

Guenter

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

On Tue, Jul 11, 2017 at 8:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>
> The hwmon warnings are all about supporting no more than 9,999 sensors
> (applesmc) to 999,999,999 sensors (scpi) of a given type.

Yeah, I think that's enough.

> Easy "fix" would be to replace snprintf() with scnprintf(), presumably
> because gcc doesn't know about scnprintf().

If that's the case, I'd prefer just turning off the format-truncation
(but not overflow) warning with '-Wno-format-trunction".

But maybe we can at least start it on a subsystem-by-subsystem basis
after people have verified their own subsusystem?

                  Linus

Re: Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

On Tue, Jul 11, 2017 at 8:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>
> The hwmon warnings are all about supporting no more than 9,999 sensors
> (applesmc) to 999,999,999 sensors (scpi) of a given type.

Yeah, I think that's enough.

> Easy "fix" would be to replace snprintf() with scnprintf(), presumably
> because gcc doesn't know about scnprintf().

If that's the case, I'd prefer just turning off the format-truncation
(but not overflow) warning with '-Wno-format-trunction".

But maybe we can at least start it on a subsystem-by-subsystem basis
after people have verified their own subsusystem?

                  Linus

Re: Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

On Tue, Jul 11, 2017 at 8:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>
> The hwmon warnings are all about supporting no more than 9,999 sensors
> (applesmc) to 999,999,999 sensors (scpi) of a given type.

Yeah, I think that's enough.

> Easy "fix" would be to replace snprintf() with scnprintf(), presumably
> because gcc doesn't know about scnprintf().

If that's the case, I'd prefer just turning off the format-truncation
(but not overflow) warning with '-Wno-format-trunction".

But maybe we can at least start it on a subsystem-by-subsystem basis
after people have verified their own subsusystem?

                  Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

On Tue, Jul 11, 2017 at 8:17 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> If that's the case, I'd prefer just turning off the format-truncation
> (but not overflow) warning with '-Wno-format-trunction".

Doing

 KBUILD_CFLAGS  += $(call cc-disable-warning, format-truncation)

in the main Makefile certainly cuts down on the warnings.

We still have some overflow warnings, including the crazy one where
gcc doesn't see that the number of max7315 boards is very limited.

But those could easily be converted to just snprintf() instead, and
then the truncation warning disabling takes care of it. Maybe that's
the right answer.

We also have about a bazillion

    warning: ‘*’ in boolean context, suggest ‘&&’ instead

warnings in drivers/ata/libata-core.c, all due to a single macro that
uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
debatable, but I suspect the macro could easily be changed too.

Tejun, would you hate just moving the "multiply by 1000" part _into_
that EZ() macro? Something like the attached (UNTESTED!) patch?

              Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1404 bytes --]

 drivers/ata/libata-core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8453f9a4682f..4c7d5a138495 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3231,19 +3231,19 @@ static const struct ata_timing ata_timing[] = {
 };
 
 #define ENOUGH(v, unit)		(((v)-1)/(unit)+1)
-#define EZ(v, unit)		((v)?ENOUGH(v, unit):0)
+#define EZ(v, unit)		((v)?ENOUGH((v)*1000, unit):0)
 
 static void ata_timing_quantize(const struct ata_timing *t, struct ata_timing *q, int T, int UT)
 {
-	q->setup	= EZ(t->setup      * 1000,  T);
-	q->act8b	= EZ(t->act8b      * 1000,  T);
-	q->rec8b	= EZ(t->rec8b      * 1000,  T);
-	q->cyc8b	= EZ(t->cyc8b      * 1000,  T);
-	q->active	= EZ(t->active     * 1000,  T);
-	q->recover	= EZ(t->recover    * 1000,  T);
-	q->dmack_hold	= EZ(t->dmack_hold * 1000,  T);
-	q->cycle	= EZ(t->cycle      * 1000,  T);
-	q->udma		= EZ(t->udma       * 1000, UT);
+	q->setup	= EZ(t->setup,      T);
+	q->act8b	= EZ(t->act8b,      T);
+	q->rec8b	= EZ(t->rec8b,      T);
+	q->cyc8b	= EZ(t->cyc8b,      T);
+	q->active	= EZ(t->active,     T);
+	q->recover	= EZ(t->recover,    T);
+	q->dmack_hold	= EZ(t->dmack_hold, T);
+	q->cycle	= EZ(t->cycle,      T);
+	q->udma		= EZ(t->udma,       UT);
 }
 
 void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,

Re: Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

On Tue, Jul 11, 2017 at 8:17 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> If that's the case, I'd prefer just turning off the format-truncation
> (but not overflow) warning with '-Wno-format-trunction".

Doing

 KBUILD_CFLAGS  += $(call cc-disable-warning, format-truncation)

in the main Makefile certainly cuts down on the warnings.

We still have some overflow warnings, including the crazy one where
gcc doesn't see that the number of max7315 boards is very limited.

But those could easily be converted to just snprintf() instead, and
then the truncation warning disabling takes care of it. Maybe that's
the right answer.

We also have about a bazillion

    warning: ‘*’ in boolean context, suggest ‘&&’ instead

warnings in drivers/ata/libata-core.c, all due to a single macro that
uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
debatable, but I suspect the macro could easily be changed too.

Tejun, would you hate just moving the "multiply by 1000" part _into_
that EZ() macro? Something like the attached (UNTESTED!) patch?

              Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1404 bytes --]

 drivers/ata/libata-core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8453f9a4682f..4c7d5a138495 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3231,19 +3231,19 @@ static const struct ata_timing ata_timing[] = {
 };
 
 #define ENOUGH(v, unit)		(((v)-1)/(unit)+1)
-#define EZ(v, unit)		((v)?ENOUGH(v, unit):0)
+#define EZ(v, unit)		((v)?ENOUGH((v)*1000, unit):0)
 
 static void ata_timing_quantize(const struct ata_timing *t, struct ata_timing *q, int T, int UT)
 {
-	q->setup	= EZ(t->setup      * 1000,  T);
-	q->act8b	= EZ(t->act8b      * 1000,  T);
-	q->rec8b	= EZ(t->rec8b      * 1000,  T);
-	q->cyc8b	= EZ(t->cyc8b      * 1000,  T);
-	q->active	= EZ(t->active     * 1000,  T);
-	q->recover	= EZ(t->recover    * 1000,  T);
-	q->dmack_hold	= EZ(t->dmack_hold * 1000,  T);
-	q->cycle	= EZ(t->cycle      * 1000,  T);
-	q->udma		= EZ(t->udma       * 1000, UT);
+	q->setup	= EZ(t->setup,      T);
+	q->act8b	= EZ(t->act8b,      T);
+	q->rec8b	= EZ(t->rec8b,      T);
+	q->cyc8b	= EZ(t->cyc8b,      T);
+	q->active	= EZ(t->active,     T);
+	q->recover	= EZ(t->recover,    T);
+	q->dmack_hold	= EZ(t->dmack_hold, T);
+	q->cycle	= EZ(t->cycle,      T);
+	q->udma		= EZ(t->udma,       UT);
 }
 
 void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,

Re: Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

On Tue, Jul 11, 2017 at 8:17 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> If that's the case, I'd prefer just turning off the format-truncation
> (but not overflow) warning with '-Wno-format-trunction".

Doing

 KBUILD_CFLAGS  += $(call cc-disable-warning, format-truncation)

in the main Makefile certainly cuts down on the warnings.

We still have some overflow warnings, including the crazy one where
gcc doesn't see that the number of max7315 boards is very limited.

But those could easily be converted to just snprintf() instead, and
then the truncation warning disabling takes care of it. Maybe that's
the right answer.

We also have about a bazillion

    warning: ‘*’ in boolean context, suggest ‘&&’ instead

warnings in drivers/ata/libata-core.c, all due to a single macro that
uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
debatable, but I suspect the macro could easily be changed too.

Tejun, would you hate just moving the "multiply by 1000" part _into_
that EZ() macro? Something like the attached (UNTESTED!) patch?

              Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1404 bytes --]

 drivers/ata/libata-core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8453f9a4682f..4c7d5a138495 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3231,19 +3231,19 @@ static const struct ata_timing ata_timing[] = {
 };
 
 #define ENOUGH(v, unit)		(((v)-1)/(unit)+1)
-#define EZ(v, unit)		((v)?ENOUGH(v, unit):0)
+#define EZ(v, unit)		((v)?ENOUGH((v)*1000, unit):0)
 
 static void ata_timing_quantize(const struct ata_timing *t, struct ata_timing *q, int T, int UT)
 {
-	q->setup	= EZ(t->setup      * 1000,  T);
-	q->act8b	= EZ(t->act8b      * 1000,  T);
-	q->rec8b	= EZ(t->rec8b      * 1000,  T);
-	q->cyc8b	= EZ(t->cyc8b      * 1000,  T);
-	q->active	= EZ(t->active     * 1000,  T);
-	q->recover	= EZ(t->recover    * 1000,  T);
-	q->dmack_hold	= EZ(t->dmack_hold * 1000,  T);
-	q->cycle	= EZ(t->cycle      * 1000,  T);
-	q->udma		= EZ(t->udma       * 1000, UT);
+	q->setup	= EZ(t->setup,      T);
+	q->act8b	= EZ(t->act8b,      T);
+	q->rec8b	= EZ(t->rec8b,      T);
+	q->cyc8b	= EZ(t->cyc8b,      T);
+	q->active	= EZ(t->active,     T);
+	q->recover	= EZ(t->recover,    T);
+	q->dmack_hold	= EZ(t->dmack_hold, T);
+	q->cycle	= EZ(t->cycle,      T);
+	q->udma		= EZ(t->udma,       UT);
 }
 
 void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Jakub Kicinski]

On Tue, 11 Jul 2017 15:35:15 -0700, Linus Torvalds wrote:
> I do suspect I'll make "-Wformat-truncation" (as opposed to
> "-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
> many of these we can fix, ok?

Somehow related - what's the stand on -Wimplicit-fallthrough?  I run
into the jump tables in jhash.h generating lots of warnings.  Is it OK
to do this?

--->8------

diff --git a/include/linux/jhash.h b/include/linux/jhash.h
index 348c6f47e4cc..f6d6513a4c03 100644
--- a/include/linux/jhash.h
+++ b/include/linux/jhash.h
@@ -85,20 +85,19 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
 		k += 12;
 	}
 	/* Last block: affect all 32 bits of (c) */
-	/* All the case statements fall through */
 	switch (length) {
-	case 12: c += (u32)k[11]<<24;
-	case 11: c += (u32)k[10]<<16;
-	case 10: c += (u32)k[9]<<8;
-	case 9:  c += k[8];
-	case 8:  b += (u32)k[7]<<24;
-	case 7:  b += (u32)k[6]<<16;
-	case 6:  b += (u32)k[5]<<8;
-	case 5:  b += k[4];
-	case 4:  a += (u32)k[3]<<24;
-	case 3:  a += (u32)k[2]<<16;
-	case 2:  a += (u32)k[1]<<8;
-	case 1:  a += k[0];
+	case 12: c += (u32)k[11]<<24;	/* fall through */
+	case 11: c += (u32)k[10]<<16;	/* fall through */
+	case 10: c += (u32)k[9]<<8;	/* fall through */
+	case 9:  c += k[8];		/* fall through */
+	case 8:  b += (u32)k[7]<<24;	/* fall through */
+	case 7:  b += (u32)k[6]<<16;	/* fall through */
+	case 6:  b += (u32)k[5]<<8;	/* fall through */
+	case 5:  b += k[4];		/* fall through */
+	case 4:  a += (u32)k[3]<<24;	/* fall through */
+	case 3:  a += (u32)k[2]<<16;	/* fall through */
+	case 2:  a += (u32)k[1]<<8;	/* fall through */
+	case 1:  a += k[0];		/* fall through */
 		 __jhash_final(a, b, c);
 	case 0: /* Nothing left to add */
 		break;
@@ -131,11 +130,11 @@ static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
 		k += 3;
 	}
 
-	/* Handle the last 3 u32's: all the case statements fall through */
+	/* Handle the last 3 u32's */
 	switch (length) {
-	case 3: c += k[2];
-	case 2: b += k[1];
-	case 1: a += k[0];
+	case 3: c += k[2];	/* fall through */
+	case 2: b += k[1];	/* fall through */
+	case 1: a += k[0];	/* fall through */
 		__jhash_final(a, b, c);
 	case 0:	/* Nothing left to add */
 		break;

Re: Lots of new warnings with gcc-7.1.1

[Jakub Kicinski]

On Tue, 11 Jul 2017 15:35:15 -0700, Linus Torvalds wrote:
> I do suspect I'll make "-Wformat-truncation" (as opposed to
> "-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
> many of these we can fix, ok?

Somehow related - what's the stand on -Wimplicit-fallthrough?  I run
into the jump tables in jhash.h generating lots of warnings.  Is it OK
to do this?

--->8------

diff --git a/include/linux/jhash.h b/include/linux/jhash.h
index 348c6f47e4cc..f6d6513a4c03 100644
--- a/include/linux/jhash.h
+++ b/include/linux/jhash.h
@@ -85,20 +85,19 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
 		k += 12;
 	}
 	/* Last block: affect all 32 bits of (c) */
-	/* All the case statements fall through */
 	switch (length) {
-	case 12: c += (u32)k[11]<<24;
-	case 11: c += (u32)k[10]<<16;
-	case 10: c += (u32)k[9]<<8;
-	case 9:  c += k[8];
-	case 8:  b += (u32)k[7]<<24;
-	case 7:  b += (u32)k[6]<<16;
-	case 6:  b += (u32)k[5]<<8;
-	case 5:  b += k[4];
-	case 4:  a += (u32)k[3]<<24;
-	case 3:  a += (u32)k[2]<<16;
-	case 2:  a += (u32)k[1]<<8;
-	case 1:  a += k[0];
+	case 12: c += (u32)k[11]<<24;	/* fall through */
+	case 11: c += (u32)k[10]<<16;	/* fall through */
+	case 10: c += (u32)k[9]<<8;	/* fall through */
+	case 9:  c += k[8];		/* fall through */
+	case 8:  b += (u32)k[7]<<24;	/* fall through */
+	case 7:  b += (u32)k[6]<<16;	/* fall through */
+	case 6:  b += (u32)k[5]<<8;	/* fall through */
+	case 5:  b += k[4];		/* fall through */
+	case 4:  a += (u32)k[3]<<24;	/* fall through */
+	case 3:  a += (u32)k[2]<<16;	/* fall through */
+	case 2:  a += (u32)k[1]<<8;	/* fall through */
+	case 1:  a += k[0];		/* fall through */
 		 __jhash_final(a, b, c);
 	case 0: /* Nothing left to add */
 		break;
@@ -131,11 +130,11 @@ static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
 		k += 3;
 	}
 
-	/* Handle the last 3 u32's: all the case statements fall through */
+	/* Handle the last 3 u32's */
 	switch (length) {
-	case 3: c += k[2];
-	case 2: b += k[1];
-	case 1: a += k[0];
+	case 3: c += k[2];	/* fall through */
+	case 2: b += k[1];	/* fall through */
+	case 1: a += k[0];	/* fall through */
 		__jhash_final(a, b, c);
 	case 0:	/* Nothing left to add */
 		break;

Re: Lots of new warnings with gcc-7.1.1

[Jakub Kicinski]

On Tue, 11 Jul 2017 15:35:15 -0700, Linus Torvalds wrote:
> I do suspect I'll make "-Wformat-truncation" (as opposed to
> "-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
> many of these we can fix, ok?

Somehow related - what's the stand on -Wimplicit-fallthrough?  I run
into the jump tables in jhash.h generating lots of warnings.  Is it OK
to do this?

--->8------

diff --git a/include/linux/jhash.h b/include/linux/jhash.h
index 348c6f47e4cc..f6d6513a4c03 100644
--- a/include/linux/jhash.h
+++ b/include/linux/jhash.h
@@ -85,20 +85,19 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
 		k += 12;
 	}
 	/* Last block: affect all 32 bits of (c) */
-	/* All the case statements fall through */
 	switch (length) {
-	case 12: c += (u32)k[11]<<24;
-	case 11: c += (u32)k[10]<<16;
-	case 10: c += (u32)k[9]<<8;
-	case 9:  c += k[8];
-	case 8:  b += (u32)k[7]<<24;
-	case 7:  b += (u32)k[6]<<16;
-	case 6:  b += (u32)k[5]<<8;
-	case 5:  b += k[4];
-	case 4:  a += (u32)k[3]<<24;
-	case 3:  a += (u32)k[2]<<16;
-	case 2:  a += (u32)k[1]<<8;
-	case 1:  a += k[0];
+	case 12: c += (u32)k[11]<<24;	/* fall through */
+	case 11: c += (u32)k[10]<<16;	/* fall through */
+	case 10: c += (u32)k[9]<<8;	/* fall through */
+	case 9:  c += k[8];		/* fall through */
+	case 8:  b += (u32)k[7]<<24;	/* fall through */
+	case 7:  b += (u32)k[6]<<16;	/* fall through */
+	case 6:  b += (u32)k[5]<<8;	/* fall through */
+	case 5:  b += k[4];		/* fall through */
+	case 4:  a += (u32)k[3]<<24;	/* fall through */
+	case 3:  a += (u32)k[2]<<16;	/* fall through */
+	case 2:  a += (u32)k[1]<<8;	/* fall through */
+	case 1:  a += k[0];		/* fall through */
 		 __jhash_final(a, b, c);
 	case 0: /* Nothing left to add */
 		break;
@@ -131,11 +130,11 @@ static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
 		k += 3;
 	}
 
-	/* Handle the last 3 u32's: all the case statements fall through */
+	/* Handle the last 3 u32's */
 	switch (length) {
-	case 3: c += k[2];
-	case 2: b += k[1];
-	case 1: a += k[0];
+	case 3: c += k[2];	/* fall through */
+	case 2: b += k[1];	/* fall through */
+	case 1: a += k[0];	/* fall through */
 		__jhash_final(a, b, c);
 	case 0:	/* Nothing left to add */
 		break;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Mauro Carvalho Chehab]

Em Tue, 11 Jul 2017 15:35:15 -0700
Linus Torvalds <torvalds@linux-foundation.org> escreveu:

> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]

Under drivers/media, I fixed a bunch of gcc 7.1 warnings before the
merge window. While most were just noise, some actually pointed to
human errors.

Now, gcc-7.1.1 produces only 6 warnings with W=1 on x86_64 (allyesconfig), 
either due to unused-but-set-variable or unused-const-variable. I guess
both warning options are disabled by default. Anyway, I have patches
to fix them already. I'll send you later.

The atomisp staging driver is a completely different beast, with would
produce itself a huge amount of warnings. I ended by adding some
logic on drivers/staging/media/atomisp/ Makefiles to disable them:

	ccflags-y += $(call cc-disable-warning, missing-declarations)
	ccflags-y += $(call cc-disable-warning, missing-prototypes)
	ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
	ccflags-y += $(call cc-disable-warning, unused-const-variable)
	ccflags-y += $(call cc-disable-warning, suggest-attribute=format)
	ccflags-y += $(call cc-disable-warning, implicit-fallthrough)

(there's actually one patch pending related to atomisp, that I'll also
be sending you soon - meant to avoid warnings if compiled with an older
gcc version)

Thanks,
Mauro

Re: Lots of new warnings with gcc-7.1.1

[Mauro Carvalho Chehab]

Em Tue, 11 Jul 2017 15:35:15 -0700
Linus Torvalds <torvalds@linux-foundation.org> escreveu:

> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]

Under drivers/media, I fixed a bunch of gcc 7.1 warnings before the
merge window. While most were just noise, some actually pointed to
human errors.

Now, gcc-7.1.1 produces only 6 warnings with W=1 on x86_64 (allyesconfig), 
either due to unused-but-set-variable or unused-const-variable. I guess
both warning options are disabled by default. Anyway, I have patches
to fix them already. I'll send you later.

The atomisp staging driver is a completely different beast, with would
produce itself a huge amount of warnings. I ended by adding some
logic on drivers/staging/media/atomisp/ Makefiles to disable them:

	ccflags-y += $(call cc-disable-warning, missing-declarations)
	ccflags-y += $(call cc-disable-warning, missing-prototypes)
	ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
	ccflags-y += $(call cc-disable-warning, unused-const-variable)
	ccflags-y += $(call cc-disable-warning, suggest-attribute=format)
	ccflags-y += $(call cc-disable-warning, implicit-fallthrough)

(there's actually one patch pending related to atomisp, that I'll also
be sending you soon - meant to avoid warnings if compiled with an older
gcc version)

Thanks,
Mauro

Re: Lots of new warnings with gcc-7.1.1

[Mauro Carvalho Chehab]

Em Tue, 11 Jul 2017 15:35:15 -0700
Linus Torvalds <torvalds@linux-foundation.org> escreveu:

> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]

Under drivers/media, I fixed a bunch of gcc 7.1 warnings before the
merge window. While most were just noise, some actually pointed to
human errors.

Now, gcc-7.1.1 produces only 6 warnings with W=1 on x86_64 (allyesconfig), 
either due to unused-but-set-variable or unused-const-variable. I guess
both warning options are disabled by default. Anyway, I have patches
to fix them already. I'll send you later.

The atomisp staging driver is a completely different beast, with would
produce itself a huge amount of warnings. I ended by adding some
logic on drivers/staging/media/atomisp/ Makefiles to disable them:

	ccflags-y += $(call cc-disable-warning, missing-declarations)
	ccflags-y += $(call cc-disable-warning, missing-prototypes)
	ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
	ccflags-y += $(call cc-disable-warning, unused-const-variable)
	ccflags-y += $(call cc-disable-warning, suggest-attribute=format)
	ccflags-y += $(call cc-disable-warning, implicit-fallthrough)

(there's actually one patch pending related to atomisp, that I'll also
be sending you soon - meant to avoid warnings if compiled with an older
gcc version)

Thanks,
Mauro

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Greg Kroah-Hartman]

On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]
> 
> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
> 
> Which in turn means that my nice clean allmodconfig compile is not an
> unholy mess of annoying new warnings.

I asked Arnd about this the other day on IRC as I've hit this as well on
the stable releases, and it's really annoying.  He mentioned that he had
lots of these warnings fixed, but didn't push most of the changes out
yet.  Arnd, any repo with them in it that we could look at?

> Normally I hate the stupid new warnings, but this time around they are
> actually exactly the kinds of warnings you'd want to see and that are
> hard for humans to pick out errors: lots of format errors wrt limited
> buffer sizes.
> 
> At the same time, many of them *are* annoying. We have various limited
> buffers that are limited for a good reason, and some of the format
> truncation warnings are about numbers in the range {0-MAX_INT], where
> we definitely know that we don't need to worry about the really big
> ones.
> 
> After all, we're using "snprintf()" for a reason - we *want* to
> truncate if the buffer is too small.

Yeah, that's the warnings in the USB core code, we "know" this will not
happen, and we are using snprintf() for that reason as well, I don't
know how to fool gcc into the fact that it's all ok here.

> Anyway, it would be lovely if some of the more affected developers
> would take a look at gcc-7.1.1 warnings. Right now I get about three
> *thousand* lines of warnings from a "make allmodconfig" build, which
> makes them a bit overwhelming.

I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
Fedora turned more warnings on in their compiler release, I'm running
Arch here:
	$ gcc --version
	gcc (GCC) 7.1.1 20170621

thanks,

greg k-h

Re: Lots of new warnings with gcc-7.1.1

[Greg Kroah-Hartman]

On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]
> 
> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
> 
> Which in turn means that my nice clean allmodconfig compile is not an
> unholy mess of annoying new warnings.

I asked Arnd about this the other day on IRC as I've hit this as well on
the stable releases, and it's really annoying.  He mentioned that he had
lots of these warnings fixed, but didn't push most of the changes out
yet.  Arnd, any repo with them in it that we could look at?

> Normally I hate the stupid new warnings, but this time around they are
> actually exactly the kinds of warnings you'd want to see and that are
> hard for humans to pick out errors: lots of format errors wrt limited
> buffer sizes.
> 
> At the same time, many of them *are* annoying. We have various limited
> buffers that are limited for a good reason, and some of the format
> truncation warnings are about numbers in the range {0-MAX_INT], where
> we definitely know that we don't need to worry about the really big
> ones.
> 
> After all, we're using "snprintf()" for a reason - we *want* to
> truncate if the buffer is too small.

Yeah, that's the warnings in the USB core code, we "know" this will not
happen, and we are using snprintf() for that reason as well, I don't
know how to fool gcc into the fact that it's all ok here.

> Anyway, it would be lovely if some of the more affected developers
> would take a look at gcc-7.1.1 warnings. Right now I get about three
> *thousand* lines of warnings from a "make allmodconfig" build, which
> makes them a bit overwhelming.

I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
Fedora turned more warnings on in their compiler release, I'm running
Arch here:
	$ gcc --version
	gcc (GCC) 7.1.1 20170621

thanks,

greg k-h

Re: Lots of new warnings with gcc-7.1.1

[Greg Kroah-Hartman]

On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]
> 
> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
> 
> Which in turn means that my nice clean allmodconfig compile is not an
> unholy mess of annoying new warnings.

I asked Arnd about this the other day on IRC as I've hit this as well on
the stable releases, and it's really annoying.  He mentioned that he had
lots of these warnings fixed, but didn't push most of the changes out
yet.  Arnd, any repo with them in it that we could look at?

> Normally I hate the stupid new warnings, but this time around they are
> actually exactly the kinds of warnings you'd want to see and that are
> hard for humans to pick out errors: lots of format errors wrt limited
> buffer sizes.
> 
> At the same time, many of them *are* annoying. We have various limited
> buffers that are limited for a good reason, and some of the format
> truncation warnings are about numbers in the range {0-MAX_INT], where
> we definitely know that we don't need to worry about the really big
> ones.
> 
> After all, we're using "snprintf()" for a reason - we *want* to
> truncate if the buffer is too small.

Yeah, that's the warnings in the USB core code, we "know" this will not
happen, and we are using snprintf() for that reason as well, I don't
know how to fool gcc into the fact that it's all ok here.

> Anyway, it would be lovely if some of the more affected developers
> would take a look at gcc-7.1.1 warnings. Right now I get about three
> *thousand* lines of warnings from a "make allmodconfig" build, which
> makes them a bit overwhelming.

I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
Fedora turned more warnings on in their compiler release, I'm running
Arch here:
	$ gcc --version
	gcc (GCC) 7.1.1 20170621

thanks,

greg k-h

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Arnd Bergmann]

On Wed, Jul 12, 2017 at 5:41 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:

>
> We also have about a bazillion
>
>     warning: =E2=80=98*=E2=80=99 in boolean context, suggest =E2=80=98&&=
=E2=80=99 instead
>
> warnings in drivers/ata/libata-core.c, all due to a single macro that
> uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> debatable, but I suspect the macro could easily be changed too.
>
> Tejun, would you hate just moving the "multiply by 1000" part _into_
> that EZ() macro? Something like the attached (UNTESTED!) patch?

Tejun applied an almost identical patch of mine a while ago, but it seems t=
o
have gotten lost in the meantime in some rebase:

https://patchwork.kernel.org/patch/9721397/
https://patchwork.kernel.org/patch/9721399/

I guess I should have resubmitted the second patch with the suggested
improvement.

     Arnd

Re: Lots of new warnings with gcc-7.1.1

[Arnd Bergmann]

On Wed, Jul 12, 2017 at 5:41 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:

>
> We also have about a bazillion
>
>     warning: ‘*’ in boolean context, suggest ‘&&’ instead
>
> warnings in drivers/ata/libata-core.c, all due to a single macro that
> uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> debatable, but I suspect the macro could easily be changed too.
>
> Tejun, would you hate just moving the "multiply by 1000" part _into_
> that EZ() macro? Something like the attached (UNTESTED!) patch?

Tejun applied an almost identical patch of mine a while ago, but it seems to
have gotten lost in the meantime in some rebase:

https://patchwork.kernel.org/patch/9721397/
https://patchwork.kernel.org/patch/9721399/

I guess I should have resubmitted the second patch with the suggested
improvement.

     Arnd

Re: Lots of new warnings with gcc-7.1.1

[Arnd Bergmann]

On Wed, Jul 12, 2017 at 5:41 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:

>
> We also have about a bazillion
>
>     warning: ‘*’ in boolean context, suggest ‘&&’ instead
>
> warnings in drivers/ata/libata-core.c, all due to a single macro that
> uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> debatable, but I suspect the macro could easily be changed too.
>
> Tejun, would you hate just moving the "multiply by 1000" part _into_
> that EZ() macro? Something like the attached (UNTESTED!) patch?

Tejun applied an almost identical patch of mine a while ago, but it seems to
have gotten lost in the meantime in some rebase:

https://patchwork.kernel.org/patch/9721397/
https://patchwork.kernel.org/patch/9721399/

I guess I should have resubmitted the second patch with the suggested
improvement.

     Arnd

Re: Lots of new warnings with gcc-7.1.1

[Arnd Bergmann]

On Wed, Jul 12, 2017 at 5:41 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:

>
> We also have about a bazillion
>
>     warning: ‘*’ in boolean context, suggest ‘&&’ instead
>
> warnings in drivers/ata/libata-core.c, all due to a single macro that
> uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> debatable, but I suspect the macro could easily be changed too.
>
> Tejun, would you hate just moving the "multiply by 1000" part _into_
> that EZ() macro? Something like the attached (UNTESTED!) patch?

Tejun applied an almost identical patch of mine a while ago, but it seems to
have gotten lost in the meantime in some rebase:

https://patchwork.kernel.org/patch/9721397/
https://patchwork.kernel.org/patch/9721399/

I guess I should have resubmitted the second patch with the suggested
improvement.

     Arnd

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Arnd Bergmann]

On Wed, Jul 12, 2017 at 3:10 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
>> [ Very random list of maintainers and mailing lists, at least
>> partially by number of warnings generated by gcc-7.1.1 that is then
>> correlated with the get_maintainers script ]
>>
>> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
>>
>> Which in turn means that my nice clean allmodconfig compile is not an
>> unholy mess of annoying new warnings.
>
> I asked Arnd about this the other day on IRC as I've hit this as well on
> the stable releases, and it's really annoying.  He mentioned that he had
> lots of these warnings fixed, but didn't push most of the changes out
> yet.

To clarify: most of the patches I wrote ended up getting merged, but
there were a couple that I did not submit a second time after they
got dropped, but I gave up on trying to fix the new -Wformat warnings
and simply disabled them, hoping someone else would do it before me,
or that the gcc developers would find a way to reduce the false-positive
ones before the release.

>  Arnd, any repo with them in it that we could look at?

I have a private tree on my workstation that has lots of random
crap, and I rebase it all the time but normally don't publish it.

I have uploaded today's snapshot to

git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git randconfig-4.13-next

The way I work with this is helpful to catch build regressions as soon
as they happen, but not so good in finding things that I have either
submitted a patch for before and don't remember if it should be
resubmitted, or stuff that I decided I didn't want to deal with at some
point.

I was already planning to start over from scratch one of these days,
and cherry-pick+resubmit the patches that are actually required
for randconfig builds.

>> Anyway, it would be lovely if some of the more affected developers
>> would take a look at gcc-7.1.1 warnings. Right now I get about three
>> *thousand* lines of warnings from a "make allmodconfig" build, which
>> makes them a bit overwhelming.
>
> I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
> Fedora turned more warnings on in their compiler release, I'm running
> Arch here:
>         $ gcc --version
>         gcc (GCC) 7.1.1 20170621

This is what I get in today's linux-next:

$ grep error: 4.13-next-allmod-warning | sed -e 's:^.*\[-W:-W:' | sort
| uniq -c | cut -f 1 -d\] | sort -n
      1 -Werror=parentheses
      2 -Werror=tautological-compare
      2 -Werror=unused-result
     34 -Werror=format-overflow=
     41 -Werror=int-in-bool-context
    233 -Werror=format-truncation=

I'll resubmit the patches for -Wparenthese, -Wtautological-compar,
-Wunused-result and -Wint-in-bool-context that I had sent earlier,
plus a new patch to move -Wformat-truncation into W=1.

      Arnd

Re: Lots of new warnings with gcc-7.1.1

[Arnd Bergmann]

On Wed, Jul 12, 2017 at 3:10 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
>> [ Very random list of maintainers and mailing lists, at least
>> partially by number of warnings generated by gcc-7.1.1 that is then
>> correlated with the get_maintainers script ]
>>
>> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
>>
>> Which in turn means that my nice clean allmodconfig compile is not an
>> unholy mess of annoying new warnings.
>
> I asked Arnd about this the other day on IRC as I've hit this as well on
> the stable releases, and it's really annoying.  He mentioned that he had
> lots of these warnings fixed, but didn't push most of the changes out
> yet.

To clarify: most of the patches I wrote ended up getting merged, but
there were a couple that I did not submit a second time after they
got dropped, but I gave up on trying to fix the new -Wformat warnings
and simply disabled them, hoping someone else would do it before me,
or that the gcc developers would find a way to reduce the false-positive
ones before the release.

>  Arnd, any repo with them in it that we could look at?

I have a private tree on my workstation that has lots of random
crap, and I rebase it all the time but normally don't publish it.

I have uploaded today's snapshot to

git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git randconfig-4.13-next

The way I work with this is helpful to catch build regressions as soon
as they happen, but not so good in finding things that I have either
submitted a patch for before and don't remember if it should be
resubmitted, or stuff that I decided I didn't want to deal with at some
point.

I was already planning to start over from scratch one of these days,
and cherry-pick+resubmit the patches that are actually required
for randconfig builds.

>> Anyway, it would be lovely if some of the more affected developers
>> would take a look at gcc-7.1.1 warnings. Right now I get about three
>> *thousand* lines of warnings from a "make allmodconfig" build, which
>> makes them a bit overwhelming.
>
> I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
> Fedora turned more warnings on in their compiler release, I'm running
> Arch here:
>         $ gcc --version
>         gcc (GCC) 7.1.1 20170621

This is what I get in today's linux-next:

$ grep error: 4.13-next-allmod-warning | sed -e 's:^.*\[-W:-W:' | sort
| uniq -c | cut -f 1 -d\] | sort -n
      1 -Werror=parentheses
      2 -Werror=tautological-compare
      2 -Werror=unused-result
     34 -Werror=format-overflow     41 -Werror=int-in-bool-context
    233 -Werror=format-truncation
I'll resubmit the patches for -Wparenthese, -Wtautological-compar,
-Wunused-result and -Wint-in-bool-context that I had sent earlier,
plus a new patch to move -Wformat-truncation into W=1.

      Arnd

Re: Lots of new warnings with gcc-7.1.1

[Arnd Bergmann]

On Wed, Jul 12, 2017 at 3:10 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Jul 11, 2017 at 03:35:15PM -0700, Linus Torvalds wrote:
>> [ Very random list of maintainers and mailing lists, at least
>> partially by number of warnings generated by gcc-7.1.1 that is then
>> correlated with the get_maintainers script ]
>>
>> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
>>
>> Which in turn means that my nice clean allmodconfig compile is not an
>> unholy mess of annoying new warnings.
>
> I asked Arnd about this the other day on IRC as I've hit this as well on
> the stable releases, and it's really annoying.  He mentioned that he had
> lots of these warnings fixed, but didn't push most of the changes out
> yet.

To clarify: most of the patches I wrote ended up getting merged, but
there were a couple that I did not submit a second time after they
got dropped, but I gave up on trying to fix the new -Wformat warnings
and simply disabled them, hoping someone else would do it before me,
or that the gcc developers would find a way to reduce the false-positive
ones before the release.

>  Arnd, any repo with them in it that we could look at?

I have a private tree on my workstation that has lots of random
crap, and I rebase it all the time but normally don't publish it.

I have uploaded today's snapshot to

git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git randconfig-4.13-next

The way I work with this is helpful to catch build regressions as soon
as they happen, but not so good in finding things that I have either
submitted a patch for before and don't remember if it should be
resubmitted, or stuff that I decided I didn't want to deal with at some
point.

I was already planning to start over from scratch one of these days,
and cherry-pick+resubmit the patches that are actually required
for randconfig builds.

>> Anyway, it would be lovely if some of the more affected developers
>> would take a look at gcc-7.1.1 warnings. Right now I get about three
>> *thousand* lines of warnings from a "make allmodconfig" build, which
>> makes them a bit overwhelming.
>
> I only have 310 when building the 4.12.0 release with 7.1.1, I wonder if
> Fedora turned more warnings on in their compiler release, I'm running
> Arch here:
>         $ gcc --version
>         gcc (GCC) 7.1.1 20170621

This is what I get in today's linux-next:

$ grep error: 4.13-next-allmod-warning | sed -e 's:^.*\[-W:-W:' | sort
| uniq -c | cut -f 1 -d\] | sort -n
      1 -Werror=parentheses
      2 -Werror=tautological-compare
      2 -Werror=unused-result
     34 -Werror=format-overflow=
     41 -Werror=int-in-bool-context
    233 -Werror=format-truncation=

I'll resubmit the patches for -Wparenthese, -Wtautological-compar,
-Wunused-result and -Wint-in-bool-context that I had sent earlier,
plus a new patch to move -Wformat-truncation into W=1.

      Arnd

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: Lots of new warnings with gcc-7.1.1

[Tejun Heo]

Hello,

On Wed, Jul 12, 2017 at 03:31:02PM +0200, Arnd Bergmann wrote:
> > We also have about a bazillion
> >
> >     warning: ‘*’ in boolean context, suggest ‘&&’ instead
> >
> > warnings in drivers/ata/libata-core.c, all due to a single macro that
> > uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> > debatable, but I suspect the macro could easily be changed too.
> >
> > Tejun, would you hate just moving the "multiply by 1000" part _into_
> > that EZ() macro? Something like the attached (UNTESTED!) patch?
> 
> Tejun applied an almost identical patch of mine a while ago, but it seems to
> have gotten lost in the meantime in some rebase:

Yeah, I was scratching my head remembering your patch.  Sorry about
that.  It should have been routed through for-4.12-fixes.

> https://patchwork.kernel.org/patch/9721397/
> https://patchwork.kernel.org/patch/9721399/
> 
> I guess I should have resubmitted the second patch with the suggested
> improvement.

The new one looks good to me.

Thanks.

-- 
tejun

Re: Lots of new warnings with gcc-7.1.1

[Tejun Heo]

Hello,

On Wed, Jul 12, 2017 at 03:31:02PM +0200, Arnd Bergmann wrote:
> > We also have about a bazillion
> >
> >     warning: ‘*’ in boolean context, suggest ‘&&’ instead
> >
> > warnings in drivers/ata/libata-core.c, all due to a single macro that
> > uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> > debatable, but I suspect the macro could easily be changed too.
> >
> > Tejun, would you hate just moving the "multiply by 1000" part _into_
> > that EZ() macro? Something like the attached (UNTESTED!) patch?
> 
> Tejun applied an almost identical patch of mine a while ago, but it seems to
> have gotten lost in the meantime in some rebase:

Yeah, I was scratching my head remembering your patch.  Sorry about
that.  It should have been routed through for-4.12-fixes.

> https://patchwork.kernel.org/patch/9721397/
> https://patchwork.kernel.org/patch/9721399/
> 
> I guess I should have resubmitted the second patch with the suggested
> improvement.

The new one looks good to me.

Thanks.

-- 
tejun

Re: Lots of new warnings with gcc-7.1.1

[Tejun Heo]

Hello,

On Wed, Jul 12, 2017 at 03:31:02PM +0200, Arnd Bergmann wrote:
> > We also have about a bazillion
> >
> >     warning: ‘*’ in boolean context, suggest ‘&&’ instead
> >
> > warnings in drivers/ata/libata-core.c, all due to a single macro that
> > uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
> > debatable, but I suspect the macro could easily be changed too.
> >
> > Tejun, would you hate just moving the "multiply by 1000" part _into_
> > that EZ() macro? Something like the attached (UNTESTED!) patch?
> 
> Tejun applied an almost identical patch of mine a while ago, but it seems to
> have gotten lost in the meantime in some rebase:

Yeah, I was scratching my head remembering your patch.  Sorry about
that.  It should have been routed through for-4.12-fixes.

> https://patchwork.kernel.org/patch/9721397/
> https://patchwork.kernel.org/patch/9721399/
> 
> I guess I should have resubmitted the second patch with the suggested
> improvement.

The new one looks good to me.

Thanks.

-- 
tejun

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Lots of new warnings with gcc-7.1.1

[Linus Torvalds]

[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]

So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1

Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.

Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.

At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.

After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.

But a lot of the warnings look reasonable, and at least the warnings
are nice in how they actually explain why the warning is happening.
Example:

  arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In
function ‘max7315_platform_data’:
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:35:
warning: ‘%d’ directive writing between 1 and 11 bytes into a region
of size 9 [-Wformat-overflow=]
     sprintf(base_pin_name, "max7315_%d_base", nr);
                                     ^~
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26:
note: directive argument in the range [-2147483647, 2147483647]

Yeah, the compiler is technically correct, but we already made sure we
have at most MAX7315_NUM of those adapters, so no, "nr" is really not
going to be a 10-digit number.

So the warning is kind of bogus.

At the same time, others aren't quite as insane, and in many cases the
warnings might be easy to just fix.

And some actually look valid, although they might still require odd input:

  net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
  net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
truncated before the last format character [-Wformat-truncation=]
    snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
                               ^~~~~~~
  net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
bytes into a destination of size 4

yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
really does need 5 bytes for "%2u\n" with the terminating NUL
character.

Of course, the "%2d" implies that people expect it to be < 100, but at
the same time it doesn't sound like a bad idea to just make the buffer
be one byte bigger. So..

Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.

I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=1" kind of warning.  But let's see how
many of these we can fix, ok?

                  Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel