On Tue, 6 Mar 2007, Thomas Gleixner wrote:
quoted text
> 
> /me pleads guilty !
> 
> Replacement patch below.

Well, I already applied the original one that came through Andrew, so I 
really just wanted to note the coding style in general, and your fixed 
patch no longer applied ;)

Also, you do end up testing for "not error" with a

quoted text
>  	if (tick_check_oneshot_change(!hrtimer_is_hres_enabled()))
> -		hrtimer_switch_to_hres();
> +		if (!hrtimer_switch_to_hres())
> +			return;

which I guess is ok, if only because we simply don't care about what the 
exact error was. But it means that this particular code sequence ends up 
having the same problem (which is still fewer places than the original 
patch, so we're good).

I personally hate the

	if (hrtimer_switch_to_hres() == SUCCESS)
		return;

kind of syntax (it's just too long, and it's *not* obvious at all that 
SUCCESS is zero and that this is a "negative error or zero" kind of 
function, so it's actually *worse* than just doing what you did, but some 
projects seem to have that kind of approach.

We could encourage people to do

	if (hrtimer_switch_to_hres() >= 0)
		return;

which is fairly obviously a "success" case for a negative error value, but 
I'm not sure the extra typing really is worth it. Does anybody have any 
smart ideas that people might even be ok with following (just making 
things more cumbersome is anti-productive, so I don't want to have some 
stupid rule that everybody really hates)?

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