Re: [PATCH 1/1] irq-gic: use BUG_ON instead of if()/BUG
From: Maninder Singh
Date: Fri Jun 19 2015 - 05:51:50 EST
Hi Thomas,
>> {
>> - if (gic_nr >= MAX_GIC_NR)
>> - BUG();
>> + BUG_ON(gic_nr >= MAX_GIC_NR);
>> if (irq_set_handler_data(irq, &gic_data[gic_nr]) != 0)
>> BUG();
>
>So this patch was clearly done just by running a script and not sanity
>checked afterwards. Otherwise the next if() BUG(); construct would
>have been fixed as well.
Yes semantic patch did the changes to use preferred APIs,
And it also changed this BUG_ON(irq_set_handler_data(irq, &gic_data[gic_nr]) != 0)
But we have to take care that condition has no side effects i.e.
if()/BUG conversion to BUG_ON must be avoided when there's side effect
in condition. The reason being BUG_ON won't execute the condition when CONFIG_BUG
is not defined As suggested by Julia Lawall
Thats why did not take that change --> (BUG_ON(irq_set_handler_data(irq, &gic_data[gic_nr]) != 0))
>Further, while we are at that. It would be even more useful to analyze
>whether the BUG_ON() is needed in the first place or at least could be
>made conditional on some debug option.
>
>But that's not done by the script either, right?
Yes coccinelle semantic patches did not do that changes.
we have to choose whether to make BUG_ON conditional on some debug options.
Thanks,
Maninder
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü¨}©ž²Æ zÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹®w¥¢¸?™¨èÚ&¢)ߢf”ù^jÇ«y§m…á@A«a¶Úÿ0¶ìh®å’i