Whether or not the sparse warning
warning: do-while statement is not a compound statement
is justified or not in this case, it is annoying and
trivial to fix.
Signed-off-by: Johannes Berg <[email protected]>
---
include/linux/kmemcheck.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- wireless-testing.orig/include/linux/kmemcheck.h 2009-07-06 11:41:16.000000000 +0200
+++ wireless-testing/include/linux/kmemcheck.h 2009-07-06 11:41:30.000000000 +0200
@@ -137,13 +137,13 @@ static inline void kmemcheck_mark_initia
int name##_end[0];
#define kmemcheck_annotate_bitfield(ptr, name) \
- do if (ptr) { \
+ do { if (ptr) { \
int _n = (long) &((ptr)->name##_end) \
- (long) &((ptr)->name##_begin); \
BUILD_BUG_ON(_n < 0); \
\
kmemcheck_mark_initialized(&((ptr)->name##_begin), _n); \
- } while (0)
+ } } while (0)
#define kmemcheck_annotate_variable(var) \
do { \
Johannes Berg wrote:
> Whether or not the sparse warning
>
> warning: do-while statement is not a compound statement
>
> is justified or not in this case, it is annoying and
> trivial to fix.
>
> Signed-off-by: Johannes Berg <[email protected]>
Looks good to me!
Acked-by: Pekka Enberg <[email protected]>
> ---
> include/linux/kmemcheck.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- wireless-testing.orig/include/linux/kmemcheck.h 2009-07-06 11:41:16.000000000 +0200
> +++ wireless-testing/include/linux/kmemcheck.h 2009-07-06 11:41:30.000000000 +0200
> @@ -137,13 +137,13 @@ static inline void kmemcheck_mark_initia
> int name##_end[0];
>
> #define kmemcheck_annotate_bitfield(ptr, name) \
> - do if (ptr) { \
> + do { if (ptr) { \
> int _n = (long) &((ptr)->name##_end) \
> - (long) &((ptr)->name##_begin); \
> BUILD_BUG_ON(_n < 0); \
> \
> kmemcheck_mark_initialized(&((ptr)->name##_begin), _n); \
> - } while (0)
> + } } while (0)
>
> #define kmemcheck_annotate_variable(var) \
> do { \
>
>
2009/7/6 Johannes Berg <[email protected]>:> Whether or not the sparse warning>> warning: do-while statement is not a compound statement>> is justified or not in this case, it is annoying and> trivial to fix.>> Signed-off-by: Johannes Berg <[email protected]>> ---> include/linux/kmemcheck.h | 4 ++--> 1 file changed, 2 insertions(+), 2 deletions(-)>> --- wireless-testing.orig/include/linux/kmemcheck.h 2009-07-06 11:41:16.000000000 +0200> +++ wireless-testing/include/linux/kmemcheck.h 2009-07-06 11:41:30.000000000 +0200> @@ -137,13 +137,13 @@ static inline void kmemcheck_mark_initia> int name##_end[0];>> #define kmemcheck_annotate_bitfield(ptr, name) \> - do if (ptr) { \> + do { if (ptr) { \> int _n = (long) &((ptr)->name##_end) \> - (long) &((ptr)->name##_begin); \> BUILD_BUG_ON(_n < 0); \> \> kmemcheck_mark_initialized(&((ptr)->name##_begin), _n); \> - } while (0)> + } } while (0)>> #define kmemcheck_annotate_variable(var) \> do { \>>>
I'll change the patch title to "kmemcheck: work around bogus sparsewarning" and fix the indentation, sounds ok?
Meanwhile, I Cced sparse mailing list in case somebody else knowsanything else about this warning (what it means, whether it'sjustified in this case, whether it should be fixed in sparse, etc.).
Thanks.
Vegard????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Wed, 2009-07-08 at 21:28 +0200, Vegard Nossum wrote:
> > #define kmemcheck_annotate_bitfield(ptr, name) \
> > - do if (ptr) { \
> > + do { if (ptr) { \
> > int _n = (long) &((ptr)->name##_end) \
> > - (long) &((ptr)->name##_begin); \
> > BUILD_BUG_ON(_n < 0); \
> > \
> > kmemcheck_mark_initialized(&((ptr)->name##_begin), _n); \
> > - } while (0)
> > + } } while (0)
> >
> > #define kmemcheck_annotate_variable(var) \
> > do { \
> >
> >
> >
>
> I'll change the patch title to "kmemcheck: work around bogus sparse
> warning" and fix the indentation, sounds ok?
Well, it's debatable whether it's bogus or not, but I don't care as long
as it gets fixed.
> Meanwhile, I Cced sparse mailing list in case somebody else knows
> anything else about this warning (what it means, whether it's
> justified in this case, whether it should be fixed in sparse, etc.).
Everybody take me off the CC please then :) This has been discussed far
too many times already. In this special case, the warning probably isn't
all that useful, but ISTR Linus saying that he wanted it this way.
johannes
On Wed, Jul 8, 2009 at 12:39 PM, Johannes Berg<[email protected]> wrote:
> Everybody take me off the CC please then :) This has been discussed far
> too many times already. In this special case, the warning probably isn't
> all that useful, but ISTR Linus saying that he wanted it this way.
Right. I would keep the warning in sparse. I think it is better with
the compound statement. It is easier to read which block is which.
> - do if (ptr) { \
> + do { if (ptr) { \
BTW, if does not hurt to put the "if (ptr)" to a separate line.
Chris
[Adding Linus and Chris Li to CC; Linus for further background on
-Wdo-while, and Chris Li for Sparse.]
On Wed, Jul 08, 2009 at 09:28:24PM +0200, Vegard Nossum wrote:
> 2009/7/6 Johannes Berg <[email protected]>:
> > Whether or not the sparse warning
> >
> > warning: do-while statement is not a compound statement
> >
> > is justified or not in this case, it is annoying and
> > trivial to fix.
[...]
>
> I'll change the patch title to "kmemcheck: work around bogus sparse
> warning" and fix the indentation, sounds ok?
>
> Meanwhile, I Cced sparse mailing list in case somebody else knows
> anything else about this warning (what it means, whether it's
> justified in this case, whether it should be fixed in sparse, etc.).
-Wdo-while gives a warning if you write:
do
statement
while (...);
where "statement" does not consist of a compound statement surrounded by
braces. As far as I know, this warning exists primarily because it
matched Linus's preference for readability.
However, note that Sparse does not have -Wdo-while enabled by default.
Sparse only issues that warning if you use -Wdo-while explicitly, or if
you pass -Wall. (The latter often happens due to passing the same
warning flags to GCC and Sparse, without using cgcc which filters out
-Wall for Sparse for that reason.)
Much discussion has occurred previously suggesting that Sparse's -Wall
should match GCC's "all useful warnings" rather than "all possible
warnings", and some other option should exist for "all possible
warnings" (for instance, -Weverything). This seems very reasonable.
Given that Sparse *exists* to give warnings, -Wall's "all useful
warnings" approach seems like it matches Sparse's default behavior;
thus, I think it would make the most sense to make Sparse just interpret
-Wall as "enable all the warnings enabled by default" (which would
normally act as a no-op, but would make a difference if you passed
"-Wno-foo -Wall" for some default warning foo).
This would improve Sparse's behavior for the case where you invoke
sparse $(CFLAGS) directly rather than setting CC=cgcc, if CFLAGS
contains -Wall.
- Josh Triplett
On Thu, Jul 9, 2009 at 08:29, Josh Triplett<[email protected]> wrote:
> [Adding Linus and Chris Li to CC; Linus for further background on
> -Wdo-while, and Chris Li for Sparse.]
>
> On Wed, Jul 08, 2009 at 09:28:24PM +0200, Vegard Nossum wrote:
>> 2009/7/6 Johannes Berg <[email protected]>:
>> > Whether or not the sparse warning
>> >
>> > warning: do-while statement is not a compound statement
>> >
>> > is justified or not in this case, it is annoying and
>> > trivial to fix.
> [...]
>>
>> I'll change the patch title to "kmemcheck: work around bogus sparse
>> warning" and fix the indentation, sounds ok?
>>
>> Meanwhile, I Cced sparse mailing list in case somebody else knows
>> anything else about this warning (what it means, whether it's
>> justified in this case, whether it should be fixed in sparse, etc.).
>
> -Wdo-while gives a warning if you write:
>
> do
> statement
> while (...);
>
> where "statement" does not consist of a compound statement surrounded by
> braces. As far as I know, this warning exists primarily because it
> matched Linus's preference for readability.
see:
http://lkml.org/lkml/2008/12/23/180
an related messages/threads
Cheers,
-Hannes
On Mon, 2009-07-06 at 11:53 +0200, Johannes Berg wrote:
> Whether or not the sparse warning
>
> warning: do-while statement is not a compound statement
>
> is justified or not in this case, it is annoying and
> trivial to fix.
Whatever happened to this patch? Did you decide to screw sparse
warnings?
johannes
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> include/linux/kmemcheck.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- wireless-testing.orig/include/linux/kmemcheck.h 2009-07-06 11:41:16.000000000 +0200
> +++ wireless-testing/include/linux/kmemcheck.h 2009-07-06 11:41:30.000000000 +0200
> @@ -137,13 +137,13 @@ static inline void kmemcheck_mark_initia
> int name##_end[0];
>
> #define kmemcheck_annotate_bitfield(ptr, name) \
> - do if (ptr) { \
> + do { if (ptr) { \
> int _n = (long) &((ptr)->name##_end) \
> - (long) &((ptr)->name##_begin); \
> BUILD_BUG_ON(_n < 0); \
> \
> kmemcheck_mark_initialized(&((ptr)->name##_begin), _n); \
> - } while (0)
> + } } while (0)
>
> #define kmemcheck_annotate_variable(var) \
> do { \
>