On Mon, May 09, 2011 at 01:31:10PM -0700, Christopher Li wrote: > On Mon, May 9, 2011 at 1:02 PM, Ben Pfaff <blp@xxxxxxxxxx> 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? I'm a developer on the Open vSwitch project (http://openvswitch.org). Open vSwitch has both kernel and userspace code. For a long time, we've been using sparse to sanity-check our kernel code. Now, I'm adding support for sanity-checking of userspace code using the same C=1 "make" option as the kernel. (There's a patch series posted starting at http://openvswitch.org/pipermail/dev/2011-May/008607.html in case you're really curious.) It's already found some bugs, mostly due to the ability to mark network byte order types as special through __attribute__((bitwise)). This warning (formerly error) is the only sparse warning left in the build, which is otherwise clean. The context for the warning is a C file of code generated by database interface definition language (IDL) bindings. Each of these structs corresponds to a row in a database table. Here's the simplest struct in question, for a database table with only two columns, named 'fault' and 'mpid': /* Maintenance_Point table. */ struct ovsrec_maintenance_point { struct ovsdb_idl_row header_; /* fault column. */ bool *fault; size_t n_fault; /* mpid column. */ int64_t mpid; }; The 'fault' member is 3-valued: a null pointer means that the row has an empty "fault" column; otherwise it points either to a malloc()'d true or false value. The warning then crops up in the generated code for populating this struct, which does something similar to the following when the "fault" column is nonempty: row->fault = xmalloc(sizeof *row->fault); *row->fault = /* value parsed from database row */; I could change my IDL code generator to do something different for this case, but I don't see anything actually wrong with it. This userspace code is not performance-critical or sensitive to memory usage, so it's not necessary on the face of it to optimize it. Thanks, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
