Date: Fri, 19 Jun 2015 09:51:36 +0000 (GMT)
From: Maninder Singh <maninder1.s@samsung.com>
Subject: Re: [PATCH 1/1] irq-gic: use BUG_ON instead of if()/BUG
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>, LKML <linux-kernel@vger.kernel.org>,
        PANKAJ MISHRA <pankaj.m@samsung.com>,
        Marc Zyngier <marc.zyngier@arm.com>
Reply-to: maninder1.s@samsung.com
MIME-version: 1.0
Msgkey: 20150619094029531@maninder1.s
Content-type: text/plain; charset=windows-1252
MIME-version: 1.0
Message-id: <814009118.53061434707495088.JavaMail.weblogic@ep2mlwas07a>
DLP-Filter: Pass
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 1396
Lines: 37

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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?