block: new gcc-5.1 warnings..

[Linus Torvalds]

So gcc-5.1 seems to have a few new warnings, most of which seem of
dubious value, but whatever.

One of them

drivers/block/hd.c: In function âhd_requestâ:
drivers/block/hd.c:630:11: warning: switch condition has boolean value
[-Wswitch-bool]
   switch (rq_data_dir(req)) {
           ^

just made me go "what?" since doing a switch on a boolean is perfectly
fine, and there can be various valid reasons to do so (using "break"
and fall-through etc can make the structure of the true/false cases
nicer).

So the compiler warning is just silly and stupid.

The warning would make more sense (and still trigger for this kernel
case) if the case statements then didn't use boolean values.

So despite the warning in general just being insane, in this case it
happens to show an oddity of the kernel source code: rq_data_dir()
returns a boolean, and that actually makes little sense, since it's
normally compared to READ/WRITE. Which *happen* to be 0/1, and integer
promotion does the right thing, but if you actually look at what
READ/WRITE are, it really is 0/1, not false/true.

This odd boolean came in through commit 5953316dbf90 ("block: make
rq->cmd_flags be 64-bit") and I think that change really was
questionable. What happened was that "cmd_flags" got turned into
"u64", and that commit wants to avoid making rq_data_dir() return a
u64, because that screws up printk() and friends.

But I think it might be better off as (I didn't test this):

 #define rq_data_dir(rq)                ((int)((rq)->cmd_flags & 1))

instead, to match the type of READ/WRITE. That would also get rid of
the (bogus) warning introduced by gcc-5.1.1.

And maybe somebody could then convince the gcc people that

   switch (boolean) {
   case true:
       ...
   case false:
   }

is actually perfectly fine.  It could still complain about the truly
odd cases (which the kernel use really arguably is).

Hmm? Jens?

               Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/