| Message ID | 1393469594.24588.71.camel@joe-AO722 (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: > Allow an override to emit or not the sizeof(bool) warning > Add a description to the manpage. > > Signed-off-by: Joe Perches <joe@perches.com> Reviewed-by: Josh Triplett <josh@joshtriplett.org> > evaluate.c | 3 ++- > lib.c | 2 ++ > lib.h | 1 + > sparse.1 | 9 +++++++++ > 4 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/evaluate.c b/evaluate.c > index 6655615..a45f59b 100644 > --- a/evaluate.c > +++ b/evaluate.c > @@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct expression *expr) > } > > if (size == 1 && is_bool_type(type)) { > - warning(expr->pos, "expression using sizeof bool"); > + if (Wsizeof_bool) > + warning(expr->pos, "expression using sizeof bool"); > size = bits_in_char; > } > > diff --git a/lib.c b/lib.c > index bf3e91c..676b72e 100644 > --- a/lib.c > +++ b/lib.c > @@ -225,6 +225,7 @@ int Wone_bit_signed_bitfield = 1; > int Wparen_string = 0; > int Wptr_subtraction_blows = 0; > int Wreturn_void = 0; > +int Wsizeof_bool = 1; > int Wshadow = 0; > int Wtransparent_union = 0; > int Wtypesign = 0; > @@ -437,6 +438,7 @@ static const struct warning { > { "paren-string", &Wparen_string }, > { "ptr-subtraction-blows", &Wptr_subtraction_blows }, > { "return-void", &Wreturn_void }, > + { "sizeof-bool", &Wsizeof_bool }, > { "shadow", &Wshadow }, > { "transparent-union", &Wtransparent_union }, > { "typesign", &Wtypesign }, > diff --git a/lib.h b/lib.h > index f09b338..7e3432f 100644 > --- a/lib.h > +++ b/lib.h > @@ -119,6 +119,7 @@ extern int Wone_bit_signed_bitfield; > extern int Wparen_string; > extern int Wptr_subtraction_blows; > extern int Wreturn_void; > +extern int Wsizeof_bool; > extern int Wshadow; > extern int Wtransparent_union; > extern int Wtypesign; > diff --git a/sparse.1 b/sparse.1 > index cd6be26..b4546be 100644 > --- a/sparse.1 > +++ b/sparse.1 > @@ -288,6 +288,15 @@ programs consider this poor style, and those programs can use > Sparse does not issue these warnings by default. > . > .TP > +.B \-Wsizeof-bool > +Warn when checking the sizeof a _Bool. > + > +C99 does not specify the sizeof a _Bool. gcc uses 1. > + > +Sparse issues these warnings by default. To turn them off, use > +\fB\-Wno\-sizeof\-bool\fR. > +. > +.TP > .B \-Wshadow > Warn when declaring a symbol which shadows a declaration with the same name in > an outer scope. > -- > 1.8.1.2.459.gbcd45b4.dirty > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sparse" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/26/2014 06:58 PM, Josh Triplett wrote: > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: >> Allow an override to emit or not the sizeof(bool) warning >> Add a description to the manpage. >> >> Signed-off-by: Joe Perches <joe@perches.com> > > Reviewed-by: Josh Triplett <josh@joshtriplett.org> > I have to admit that this particular warning is a bit odd to me. I'm wondering what kind of bugs it was intended to catch. In particular, things that incorrectly assumes the size of bool to be anything in particular would seem unlikely to actually use sizeof(). -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(adding Ben Pfaff and Christopher Li) On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: > On 02/26/2014 06:58 PM, Josh Triplett wrote: > > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: > >> Allow an override to emit or not the sizeof(bool) warning > >> Add a description to the manpage. > >> > >> Signed-off-by: Joe Perches <joe@perches.com> > > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org> > > > > I have to admit that this particular warning is a bit odd to me. I'm > wondering what kind of bugs it was intended to catch. > > In particular, things that incorrectly assumes the size of bool to be > anything in particular would seem unlikely to actually use sizeof(). Dunno, the commit log for the commit that added it doesn't quite match the code and is seemingly unaware that the c99 spec doesn't specify sizeof(bool). Ben Pfaff <blp@nicira.com> Date: Wed May 4 16:39:54 2011 -0700 evaluate: Allow sizeof(_Bool) to succeed. Without this commit, sizeof(_Bool) provokes an error with "cannot size expression" because _Bool is a 1-bit type and thus not a multiple of a full byte in size. But sizeof(_Bool) is valid C that should evaluate to 1, so this commit fixes the problem and adds a regression test. Signed-off-by: Ben Pfaff <blp@nicira.com> Signed-off-by: Christopher Li <sparse@chrisli.org> -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
sizeof(_Bool), like for many other types, is ABI-dependent, but that doesn't mean it is illegitimate. I don't think C99 says that it is invalid (which means C99 doesn't permit is to be a packed bitmap.) On February 26, 2014 7:38:46 PM PST, Joe Perches <joe@perches.com> wrote: >(adding Ben Pfaff and Christopher Li) > >On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: >> On 02/26/2014 06:58 PM, Josh Triplett wrote: >> > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: >> >> Allow an override to emit or not the sizeof(bool) warning >> >> Add a description to the manpage. >> >> >> >> Signed-off-by: Joe Perches <joe@perches.com> >> > >> > Reviewed-by: Josh Triplett <josh@joshtriplett.org> >> > >> >> I have to admit that this particular warning is a bit odd to me. I'm >> wondering what kind of bugs it was intended to catch. >> >> In particular, things that incorrectly assumes the size of bool to be >> anything in particular would seem unlikely to actually use sizeof(). > >Dunno, the commit log for the commit that added it doesn't quite >match the code and is seemingly unaware that the c99 spec doesn't >specify sizeof(bool). > >Ben Pfaff <blp@nicira.com> >Date: Wed May 4 16:39:54 2011 -0700 > > evaluate: Allow sizeof(_Bool) to succeed. > > Without this commit, sizeof(_Bool) provokes an error with "cannot size >expression" because _Bool is a 1-bit type and thus not a multiple of a >full >byte in size. But sizeof(_Bool) is valid C that should evaluate to 1, >so > this commit fixes the problem and adds a regression test. > > Signed-off-by: Ben Pfaff <blp@nicira.com> > Signed-off-by: Christopher Li <sparse@chrisli.org>
On Wed, Feb 26, 2014 at 07:38:46PM -0800, Joe Perches wrote: > (adding Ben Pfaff and Christopher Li) > > On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: > > On 02/26/2014 06:58 PM, Josh Triplett wrote: > > > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: > > >> Allow an override to emit or not the sizeof(bool) warning > > >> Add a description to the manpage. > > >> > > >> Signed-off-by: Joe Perches <joe@perches.com> > > > > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org> > > > > > > > I have to admit that this particular warning is a bit odd to me. I'm > > wondering what kind of bugs it was intended to catch. > > > > In particular, things that incorrectly assumes the size of bool to be > > anything in particular would seem unlikely to actually use sizeof(). > > Dunno, the commit log for the commit that added it doesn't quite > match the code and is seemingly unaware that the c99 spec doesn't > specify sizeof(bool). The commit *relaxed* sparse behavior: because previously sizeof(bool) was an error. I'm not in favor of any diagnostic at all for sizeof(bool), but my recollection is that a sparse maintainer wanted it to yield one. I don't care about the particular result for sizeof(bool) as long as it matches the ABI. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/26/2014 08:00 PM, Ben Pfaff wrote: > > The commit *relaxed* sparse behavior: because previously sizeof(bool) > was an error. I'm not in favor of any diagnostic at all for > sizeof(bool), but my recollection is that a sparse maintainer wanted it > to yield one. > Still not clear as to why. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 26, 2014 at 08:19:57PM -0800, H. Peter Anvin wrote: > On 02/26/2014 08:00 PM, Ben Pfaff wrote: > > > > The commit *relaxed* sparse behavior: because previously sizeof(bool) > > was an error. I'm not in favor of any diagnostic at all for > > sizeof(bool), but my recollection is that a sparse maintainer wanted it > > to yield one. > > Still not clear as to why. The discussion is here: http://comments.gmane.org/gmane.comp.parsers.sparse/2462 Quoting from that discussion, the core of Christopher Li's argument was this: > On Mon, May 9, 2011 at 1:02 PM, Ben Pfaff <blp <at> nicira.com> wrote: > > Thank you for applying my patch. It does work for me, in the sense > > that I get a warning instead of an error now, but I'm not so happy to > > get any diagnostic at all. Is there some reason why sizeof(_Bool) > > warrants a warning when, say, sizeof(long) does not? After all, both > > sizes are implementation defined. > Because sizeof(_Bool) is a little bit special compare to sizeof(long). > In the case of long, all sizeof(long) * 8 bits are use in the actual value. > But for the _Bool, only the 1 bit is used in the 8 bits size. In other words, > the _Bool has a special case of the actual bit size is not a multiple of 8. > Sparse has two hats, it is a C compiler front end, and more often it is > used in the Linux kernel source sanitize checking. Depending on the sizeof > _Bool sounds a little bit suspicious in the kernel. I would love to the heard > your actual usage case of the sizeof(_Bool). Why do you care about this > warning? -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/26/2014 08:26 PM, Ben Pfaff wrote: > >> Because sizeof(_Bool) is a little bit special compare to sizeof(long). >> In the case of long, all sizeof(long) * 8 bits are use in the actual value. >> But for the _Bool, only the 1 bit is used in the 8 bits size. In other words, >> the _Bool has a special case of the actual bit size is not a multiple of 8. Quite frankly, this is silly in my opinion, *and* it is not guaranteed by C either (read about "trap representations"). >> Sparse has two hats, it is a C compiler front end, and more often it is >> used in the Linux kernel source sanitize checking. Depending on the sizeof >> _Bool sounds a little bit suspicious in the kernel. I would love to the heard >> your actual usage case of the sizeof(_Bool). Why do you care about this >> warning? Anything that moves data around in a generic fashion. It can be as simple as: memcpy(foo, bar, sizeof *foo); -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 26, 2014 at 07:42:37PM -0800, H. Peter Anvin wrote: > sizeof(_Bool), like for many other types, is ABI-dependent, but that > doesn't mean it is illegitimate. > > I don't think C99 says that it is invalid (which means C99 doesn't > permit is to be a packed bitmap.) Ok, but what can be said about the __pcpu_size_call() use case where we do sizeof(bool)? We have there accessors for sizes 1,2,4 and 8. Can we simply assume that the ABI will give us a size of bool which is one of those? What if sizeof(bool) is 3? Or, are we saying that sizeof(bool) will always be of some natural, native size like byte, short, int or long so we're good there? Thanks.
Keep in mind, too, that for the kernel we don't care about the full C standard but a subset. We rely on extrastandard behavior all over the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 and so everything is sane. On February 27, 2014 12:25:29 AM PST, Borislav Petkov <bp@alien8.de> wrote: >On Wed, Feb 26, 2014 at 07:42:37PM -0800, H. Peter Anvin wrote: >> sizeof(_Bool), like for many other types, is ABI-dependent, but that >> doesn't mean it is illegitimate. >> >> I don't think C99 says that it is invalid (which means C99 doesn't >> permit is to be a packed bitmap.) > >Ok, but what can be said about the __pcpu_size_call() use case where we >do sizeof(bool)? We have there accessors for sizes 1,2,4 and 8. Can we >simply assume that the ABI will give us a size of bool which is one of >those? > >What if sizeof(bool) is 3? > >Or, are we saying that sizeof(bool) will always be of some natural, >native size like byte, short, int or long so we're good there? > >Thanks.
On Thu, Feb 27, 2014 at 07:10:25AM -0800, H. Peter Anvin wrote: > Keep in mind, too, that for the kernel we don't care about the full > C standard but a subset. We rely on extrastandard behavior all over > the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 > and so everything is sane. Do we have a fairly comprehensive list of what these extrastandard requirements / assumptions are? It might be a good idea to have one that we can point to, so that (a) people who are trying to define a new architecture knows what they need to handle, (b) and so we can give a list of things that static code analyzers like smatch and coverity and sparse should be able to suppress (perhaps in a Linux kernel-only mode). - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/27/2014 07:24 AM, Theodore Ts'o wrote: > On Thu, Feb 27, 2014 at 07:10:25AM -0800, H. Peter Anvin wrote: >> Keep in mind, too, that for the kernel we don't care about the full >> C standard but a subset. We rely on extrastandard behavior all over >> the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 >> and so everything is sane. > > Do we have a fairly comprehensive list of what these extrastandard > requirements / assumptions are? It might be a good idea to have one > that we can point to, so that (a) people who are trying to define a > new architecture knows what they need to handle, (b) and so we can > give a list of things that static code analyzers like smatch and > coverity and sparse should be able to suppress (perhaps in a Linux > kernel-only mode). > No, but I think we can certainly make a list... a lot of it right now sits in various people's heads. Here are a couple: - Bytes are 8 bits - Signed integers will be 2's complement - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. - sizeof(long) == sizeof(void *) - NULL is represented by all zero - Structures will not add padding as long as all the members are naturally aligned. Someone want to set up a collaborative document of some kind and collect more? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 27, 2014 at 07:48:35AM -0800, H. Peter Anvin wrote: > No, but I think we can certainly make a list... a lot of it right now > sits in various people's heads. > > Here are a couple: > > - Bytes are 8 bits > - Signed integers will be 2's complement > - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or ^ ^ bool, ,1 , > 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. > - sizeof(long) == sizeof(void *) > - NULL is represented by all zero > - Structures will not add padding as long as all the members are > naturally aligned. > > Someone want to set up a collaborative document of some kind and collect > more? https://wiki.kernel.org/ but I hear spammers love it.
On Thu, Feb 27, 2014 at 07:48:35AM -0800, H. Peter Anvin wrote: > > Do we have a fairly comprehensive list of what these extrastandard > > requirements / assumptions are? It might be a good idea to have one > > that we can point to, so that (a) people who are trying to define a > > new architecture knows what they need to handle, (b) and so we can > > give a list of things that static code analyzers like smatch and > > coverity and sparse should be able to suppress (perhaps in a Linux > > kernel-only mode). > > > > No, but I think we can certainly make a list... a lot of it right now > sits in various people's heads. > > Here are a couple: > > - Bytes are 8 bits > - Signed integers will be 2's complement > - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or > 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. > - sizeof(long) == sizeof(void *) > - NULL is represented by all zero > - Structures will not add padding as long as all the members are > naturally aligned. > That last assumption has to change for the Meta architecture. https://lwn.net/Articles/522188/ On meta, the structs and unions are padded to 4 bytes unless they are explicitly marked as __packed. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/27/2014 08:10 AM, Dan Carpenter wrote: > > That last assumption has to change for the Meta architecture. > > https://lwn.net/Articles/522188/ > > On meta, the structs and unions are padded to 4 bytes unless they are > explicitly marked as __packed. > One have to wonder how likely they are to have that not continually break... -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27/02/14 16:52, H. Peter Anvin wrote: > On 02/27/2014 08:10 AM, Dan Carpenter wrote: >> >> That last assumption has to change for the Meta architecture. >> >> https://lwn.net/Articles/522188/ >> >> On meta, the structs and unions are padded to 4 bytes unless they are >> explicitly marked as __packed. >> > > One have to wonder how likely they are to have that not continually break... Well it's only when it deals with stored/portable data formats that it's an issue, and in those cases some amount of care over packing has usually already been taken. It most often causes problems when one small struct/union containing only chars or shorts is nested inside another struct which is marked as packed (on the unfortunately mistaken assumption that the inner struct cannot have padding at the end so doesn't need packing itself). But yeh, it is a PITA. Cheers James -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 26, 2014 at 8:32 PM, H. Peter Anvin <hpa@zytor.com> wrote: > > Quite frankly, this is silly in my opinion, *and* it is not guaranteed > by C either (read about "trap representations"). >> > Anything that moves data around in a generic fashion. It can be as > simple as: > > memcpy(foo, bar, sizeof *foo); OK. I get it nobody wants a sizeof(_Bool) warning. I am going to apply this patch. Should we change the default to off then? Thanks. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I would. On February 27, 2014 12:22:45 PM PST, Christopher Li <sparse@chrisli.org> wrote: >On Wed, Feb 26, 2014 at 8:32 PM, H. Peter Anvin <hpa@zytor.com> wrote: >> >> Quite frankly, this is silly in my opinion, *and* it is not >guaranteed >> by C either (read about "trap representations"). >>> >> Anything that moves data around in a generic fashion. It can be as >> simple as: >> >> memcpy(foo, bar, sizeof *foo); > >OK. I get it nobody wants a sizeof(_Bool) warning. > >I am going to apply this patch. > >Should we change the default to off then? > >Thanks. > >Chris
On Thu, 2014-02-27 at 12:26 -0800, H. Peter Anvin wrote: > On February 27, 2014 12:22:45 PM PST, Christopher Li <sparse@chrisli.org> wrote: > >OK. I get it nobody wants a sizeof(_Bool) warning. > >I am going to apply this patch. Please use V3 as I stuffed up the alphabetic order of sizeof and shadow. > >Should we change the default to off then? > I would. I'm not sure it matters much, but the linux-kernel Makefile wouldn't need to be changed if Wsizeof_bool is default 0. Here's a couple of other nits: Maybe the evaluate.c "size = bits_in_char;" assignment if (size == 1 && is_bool_type(type)) { - warning(expr->pos, "expression using sizeof bool"); + if (Wsizeof_bool) + warning(expr->pos, "expression using sizeof bool"); size = bits_in_char; } should be size = sizeof(_Bool) * 8; And also, in sparse.1, Josh Triplett is shown as the maintainer. Maybe that should be changed to Christopher Li -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 27, 2014 at 12:39 PM, Joe Perches <joe@perches.com> wrote: > Please use V3 as I stuffed up the alphabetic order > of sizeof and shadow. Please send it your V3 then :-) > > I'm not sure it matters much, but the linux-kernel > Makefile wouldn't need to be changed if Wsizeof_bool > is default 0. It seems default to off is the right thing to do. > > Here's a couple of other nits: > > Maybe the evaluate.c "size = bits_in_char;" assignment > > if (size == 1 && is_bool_type(type)) { > - warning(expr->pos, "expression using sizeof bool"); > + if (Wsizeof_bool) > + warning(expr->pos, "expression using sizeof bool"); > size = bits_in_char; > } > > should be > > size = sizeof(_Bool) * 8; The reason to use bits_in_xxxx is to allow sparse application to over write the size of int etc. If you don't like the bits_in_char here. You can introduce bits_in_bool and set that to sizeof(Bool)*8 by default. That way you don't hard code it. > And also, in sparse.1, Josh Triplett is shown as > the maintainer. Maybe that should be changed to > Christopher Li Maybe a separate patch. Waiting for your V3. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2014-02-27 at 12:55 -0800, Christopher Li wrote: > On Thu, Feb 27, 2014 at 12:39 PM, Joe Perches <joe@perches.com> wrote: > > Maybe the evaluate.c "size = bits_in_char;" assignment > > > > if (size == 1 && is_bool_type(type)) { > > - warning(expr->pos, "expression using sizeof bool"); > > + if (Wsizeof_bool) > > + warning(expr->pos, "expression using sizeof bool"); > > size = bits_in_char; > > } > > > > should be > > > > size = sizeof(_Bool) * 8; > > The reason to use bits_in_xxxx is to allow sparse application to over write > the size of int etc. If you don't like the bits_in_char here. You can introduce > bits_in_bool and set that to sizeof(Bool)*8 by default. That way you don't > hard code it. There already is a bits_in_bool and it's default 1. $ git grep -E "\bbits_in_\w+\s*=[^=]" lib.c: bits_in_long = 64; lib.c: bits_in_pointer = 64; target.c:int bits_in_bool = 1; target.c:int bits_in_char = 8; target.c:int bits_in_short = 16; target.c:int bits_in_int = 32; target.c:int bits_in_long = 32; target.c:int bits_in_longlong = 64; target.c:int bits_in_longlonglong = 128; target.c:int bits_in_float = 32; target.c:int bits_in_double = 64; target.c:int bits_in_longdouble = 80; target.c:int bits_in_pointer = 32; target.c:int bits_in_enum = 32; > > And also, in sparse.1, Josh Triplett is shown as > > the maintainer. Maybe that should be changed to > > Christopher Li > > Maybe a separate patch. That's fine with me too. If you're the maintainer, I think you should do that patch. I don't see a need for me to send any more right now though. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/evaluate.c b/evaluate.c index 6655615..a45f59b 100644 --- a/evaluate.c +++ b/evaluate.c @@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct expression *expr) } if (size == 1 && is_bool_type(type)) { - warning(expr->pos, "expression using sizeof bool"); + if (Wsizeof_bool) + warning(expr->pos, "expression using sizeof bool"); size = bits_in_char; } diff --git a/lib.c b/lib.c index bf3e91c..676b72e 100644 --- a/lib.c +++ b/lib.c @@ -225,6 +225,7 @@ int Wone_bit_signed_bitfield = 1; int Wparen_string = 0; int Wptr_subtraction_blows = 0; int Wreturn_void = 0; +int Wsizeof_bool = 1; int Wshadow = 0; int Wtransparent_union = 0; int Wtypesign = 0; @@ -437,6 +438,7 @@ static const struct warning { { "paren-string", &Wparen_string }, { "ptr-subtraction-blows", &Wptr_subtraction_blows }, { "return-void", &Wreturn_void }, + { "sizeof-bool", &Wsizeof_bool }, { "shadow", &Wshadow }, { "transparent-union", &Wtransparent_union }, { "typesign", &Wtypesign }, diff --git a/lib.h b/lib.h index f09b338..7e3432f 100644 --- a/lib.h +++ b/lib.h @@ -119,6 +119,7 @@ extern int Wone_bit_signed_bitfield; extern int Wparen_string; extern int Wptr_subtraction_blows; extern int Wreturn_void; +extern int Wsizeof_bool; extern int Wshadow; extern int Wtransparent_union; extern int Wtypesign; diff --git a/sparse.1 b/sparse.1 index cd6be26..b4546be 100644 --- a/sparse.1 +++ b/sparse.1 @@ -288,6 +288,15 @@ programs consider this poor style, and those programs can use Sparse does not issue these warnings by default. . .TP +.B \-Wsizeof-bool +Warn when checking the sizeof a _Bool. + +C99 does not specify the sizeof a _Bool. gcc uses 1. + +Sparse issues these warnings by default. To turn them off, use +\fB\-Wno\-sizeof\-bool\fR. +. +.TP .B \-Wshadow Warn when declaring a symbol which shadows a declaration with the same name in an outer scope.
Allow an override to emit or not the sizeof(bool) warning Add a description to the manpage. Signed-off-by: Joe Perches <joe@perches.com> --- evaluate.c | 3 ++- lib.c | 2 ++ lib.h | 1 + sparse.1 | 9 +++++++++ 4 files changed, 14 insertions(+), 1 deletion(-)