On Sun, Nov 4, 2012 at 11:16 AM, Julia Lawall <julia.lawall@xxxxxxx> wrote: > I didn't change any cases where the if test contains a function call. The > current definitions of WARN_ON seem to always evaluate the condition > expression, but I was worried that that might not always be the case. And > calling a function (the ones I remember were some kinds of print functions) > seems like something one might not want buried in the argument of a > debugging macro. Makes sense. > WARN_ON_SMP is just WARN_ON if CONFIG_SMP is true, but it is just 0 > otherwise. So in that case it seems important to check that one is not > throwing away something important. Yup, we just need to make sure that the expression being evaluated doesn't have side-effects. > I remember working on the BUG_ON case several years ago, and other people > worked on it too, but I guess some are still there... The current > definitions of BUG_ON seem to keep the condition, but there are quite a few > specialized definitions, so someone at some point might make a version that > does not have that property. It makes sense to keep an eye for such things when converting code. I also don't think we'll get to see a version of BUG_ON which doesn't evaluate the expression since the kernel already has more than enough BUG_ONs that assume the expression will be evaluated: BUG_ON(HYPERVISOR_callback_op(CALLBACKOP_register, &event)); BUG_ON(gpiochip_add(&gemini_gpio_chip)); BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE)); BUG_ON(gpio_request(ZOOM2_HEADSET_MUX_GPIO, "hs_mux") < 0); And so on, so we're probably safe converting to BUG_ON even if the condition is a function, as long as it doesn't create a long and complicated BUG_ON() ofcourse. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
