Re: [tip:core/locking] locking, x86: Slightly shorten__ticket_spin_trylock()

[Linus Torvalds]

On Wed, 2 Dec 2009, tip-bot for Jan Beulich wrote:
> 
> locking, x86: Slightly shorten __ticket_spin_trylock()

NAK. This is horrible crap. Don't ever send me this kind of broken patch 
again.

> -	int new;
> +	union { int i; bool b; } new;

No thank you. This is total bogosity.

You have zero idea what type "bool" is, do you? It can well be "int", it 
can be "char", it can be some compiler-internal type ("_Bool"). You have 
no idea what size it is.

And maybe it _is_ just a byte. But even if it is, using 'bool' here is 
wrong. The fact is, bool has magic semantic properties outside of sizing. 
You can't mix it with inline asm, because you simply don't know what the 
compiler rules for 'bool' are.

For example, maybe the rules are that it's always passed as an integer, 
and is always guaranteed to have the values 0/1. So even if 'sizeof' 
returns 1, that doesn't actually mean that you can necessarily pass it 
around as a char - it only means that it will take one byte in a structure 
(except that bool arrays might be packed, I think).

In other words, the semantics of 'bool' are such that you have no clue 
what the actual ABI for 'bool' is. You cannot mix this with asm.

Secondly, the notion of using a union here is just totally broken. There's 
no point to it, and it just makes the code look horrible.

So if you want to do this, then just keep 'new' as an int, and make sure 
that the function returns a 'char'. No games with 'bool' which is badly 
defined, no games with unions.

And please do make sure that it actually doesn't deprove code at the 
callers too.

				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/