On Tue, 6 Mar 2007, Thomas Gleixner wrote:
>
> /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
> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
