Re: [PATCH] evaluate: Allow sizeof(_Bool) to succeed.

[Ben Pfaff <blp@nicira.com>]

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@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?

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.