[PATCH] vmxnet3: convert BUG_ON(true) into a simple BUG()

[PATCH] vmxnet3: convert BUG_ON(true) into a simple BUG()

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/net/vmxnet3/vmxnet3_drv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 0ae1bcc..7e9622f 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -1922,7 +1922,7 @@ vmxnet3_free_irqs(struct vmxnet3_adapter *adapter)
 		free_irq(adapter->pdev->irq, adapter->netdev);
 		break;
 	default:
-		BUG_ON(true);
+		BUG();
 	}
 }
 
-- 
1.7.10.4

[PATCH] alpha: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/alpha/kernel/pci_iommu.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index 3f844d2..a21d0ab 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -354,8 +354,7 @@ static dma_addr_t alpha_pci_map_page(struct device *dev, struct page *page,
 	struct pci_dev *pdev = alpha_gendev_to_pci(dev);
 	int dac_allowed;
 
-	if (dir == PCI_DMA_NONE)
-		BUG();
+	BUG_ON(dir == PCI_DMA_NONE);
 
 	dac_allowed = pdev ? pci_dac_dma_supported(pdev, pdev->dma_mask) : 0; 
 	return pci_map_single_1(pdev, (char *)page_address(page) + offset, 
@@ -378,8 +377,7 @@ static void alpha_pci_unmap_page(struct device *dev, dma_addr_t dma_addr,
 	struct pci_iommu_arena *arena;
 	long dma_ofs, npages;
 
-	if (dir == PCI_DMA_NONE)
-		BUG();
+	BUG_ON(dir == PCI_DMA_NONE);
 
 	if (dma_addr >= __direct_map_base
 	    && dma_addr < __direct_map_base + __direct_map_size) {
@@ -662,8 +660,7 @@ static int alpha_pci_map_sg(struct device *dev, struct scatterlist *sg,
 	dma_addr_t max_dma;
 	int dac_allowed;
 
-	if (dir == PCI_DMA_NONE)
-		BUG();
+	BUG_ON(dir == PCI_DMA_NONE);
 
 	dac_allowed = dev ? pci_dac_dma_supported(pdev, pdev->dma_mask) : 0;
 
@@ -742,8 +739,7 @@ static void alpha_pci_unmap_sg(struct device *dev, struct scatterlist *sg,
 	dma_addr_t max_dma;
 	dma_addr_t fbeg, fend;
 
-	if (dir == PCI_DMA_NONE)
-		BUG();
+	BUG_ON(dir == PCI_DMA_NONE);
 
 	if (! alpha_mv.mv_pci_tbi)
 		return;
-- 
1.7.10.4

Re: [PATCH] alpha: use BUG_ON where possible

[Jiri Kosina]

On Thu, 8 Nov 2012, Sasha Levin wrote:

> Just use BUG_ON() instead of constructions such as:
> 
> 	if (...)
> 		BUG()
> 
> A simplified version of the semantic patch that makes this transformation
> is as follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e;
> @@
> - if (e) BUG();
> + BUG_ON(e);

Ok, I guess I am just going to apply this one. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] alpha: use BUG_ON where possible

[Sasha Levin]

Hi Jiri,

On 11/08/2012 04:42 PM, Jiri Kosina wrote:
> On Thu, 8 Nov 2012, Sasha Levin wrote:
> 
>> Just use BUG_ON() instead of constructions such as:
>>
>> 	if (...)
>> 		BUG()
>>
>> A simplified version of the semantic patch that makes this transformation
>> is as follows: (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @@
>> expression e;
>> @@
>> - if (e) BUG();
>> + BUG_ON(e);
> 
> Ok, I guess I am just going to apply this one. Thanks,
> 

So I have about 80 more of these. I've sent out the first 10 to make sure
that they look fine and the only complaints I got about them are that they are
trivial :)

Can I send the rest directly to you to go through the trivial tree?


Thanks,
Sasha

Re: [PATCH] alpha: use BUG_ON where possible

[Jiri Kosina]

On Fri, 9 Nov 2012, Sasha Levin wrote:

> >> Just use BUG_ON() instead of constructions such as:
> >>
> >> 	if (...)
> >> 		BUG()
> >>
> >> A simplified version of the semantic patch that makes this transformation
> >> is as follows: (http://coccinelle.lip6.fr/)
> >>
> >> // <smpl>
> >> @@
> >> expression e;
> >> @@
> >> - if (e) BUG();
> >> + BUG_ON(e);
> > 
> > Ok, I guess I am just going to apply this one. Thanks,
> > 
> 
> So I have about 80 more of these. I've sent out the first 10 to make sure
> that they look fine and the only complaints I got about them are that they are
> trivial :)
> 
> Can I send the rest directly to you to go through the trivial tree?

How many conflict can you see if you generate this patch on top of Linus' 
tree and thn apply it to linux-next?

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] alpha: use BUG_ON where possible

[Sasha Levin]

On 11/12/2012 09:43 AM, Jiri Kosina wrote:
> On Fri, 9 Nov 2012, Sasha Levin wrote:
> 
>>>> Just use BUG_ON() instead of constructions such as:
>>>>
>>>> 	if (...)
>>>> 		BUG()
>>>>
>>>> A simplified version of the semantic patch that makes this transformation
>>>> is as follows: (http://coccinelle.lip6.fr/)
>>>>
>>>> // <smpl>
>>>> @@
>>>> expression e;
>>>> @@
>>>> - if (e) BUG();
>>>> + BUG_ON(e);
>>>
>>> Ok, I guess I am just going to apply this one. Thanks,
>>>
>>
>> So I have about 80 more of these. I've sent out the first 10 to make sure
>> that they look fine and the only complaints I got about them are that they are
>> trivial :)
>>
>> Can I send the rest directly to you to go through the trivial tree?
> 
> How many conflict can you see if you generate this patch on top of Linus' 
> tree and thn apply it to linux-next?
> 

One context conflict.


Thanks,
Sasha

[PATCH] ARM: gic: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/arm/common/gic.c |   18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index aa52699..f0b8a10 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -336,10 +336,8 @@ static struct irq_chip gic_chip = {
 
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 {
-	if (gic_nr >= MAX_GIC_NR)
-		BUG();
-	if (irq_set_handler_data(irq, &gic_data[gic_nr]) != 0)
-		BUG();
+	BUG_ON(gic_nr >= MAX_GIC_NR);
+	BUG_ON(irq_set_handler_data(irq, &gic_data[gic_nr]) != 0);
 	irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
@@ -421,8 +419,7 @@ static void gic_dist_save(unsigned int gic_nr)
 	void __iomem *dist_base;
 	int i;
 
-	if (gic_nr >= MAX_GIC_NR)
-		BUG();
+	BUG_ON(gic_nr >= MAX_GIC_NR);
 
 	gic_irqs = gic_data[gic_nr].gic_irqs;
 	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
@@ -456,8 +453,7 @@ static void gic_dist_restore(unsigned int gic_nr)
 	unsigned int i;
 	void __iomem *dist_base;
 
-	if (gic_nr >= MAX_GIC_NR)
-		BUG();
+	BUG_ON(gic_nr >= MAX_GIC_NR);
 
 	gic_irqs = gic_data[gic_nr].gic_irqs;
 	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
@@ -493,8 +489,7 @@ static void gic_cpu_save(unsigned int gic_nr)
 	void __iomem *dist_base;
 	void __iomem *cpu_base;
 
-	if (gic_nr >= MAX_GIC_NR)
-		BUG();
+	BUG_ON(gic_nr >= MAX_GIC_NR);
 
 	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
 	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
@@ -519,8 +514,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 	void __iomem *dist_base;
 	void __iomem *cpu_base;
 
-	if (gic_nr >= MAX_GIC_NR)
-		BUG();
+	BUG_ON(gic_nr >= MAX_GIC_NR);
 
 	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
 	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
-- 
1.7.10.4

[PATCH] ARM: kprobes: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/arm/kernel/kprobes-test.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
index 1862d8f..0fb370d 100644
--- a/arch/arm/kernel/kprobes-test.c
+++ b/arch/arm/kernel/kprobes-test.c
@@ -1212,8 +1212,7 @@ static int register_test_probe(struct test_probe *probe)
 {
 	int ret;
 
-	if (probe->registered)
-		BUG();
+	BUG_ON(probe->registered);
 
 	ret = register_kprobe(&probe->kprobe);
 	if (ret >= 0) {
-- 
1.7.10.4

Re: [PATCH] ARM: kprobes: use BUG_ON where possible

[Jon Medhurst (Tixy)]

On Thu, 2012-11-08 at 15:23 -0500, Sasha Levin wrote:
> Just use BUG_ON() instead of constructions such as:
> 
> 	if (...)
> 		BUG()
> 
> A simplified version of the semantic patch that makes this transformation
> is as follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e;
> @@
> - if (e) BUG();
> + BUG_ON(e);
> // </smpl>
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

I'm not sure that trivial changes like this are worth it, but equally,
they're not worth having a discussion about, so...

Acked-by: Jon Medhurst <tixy@linaro.org>

> ---
>  arch/arm/kernel/kprobes-test.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
> index 1862d8f..0fb370d 100644
> --- a/arch/arm/kernel/kprobes-test.c
> +++ b/arch/arm/kernel/kprobes-test.c
> @@ -1212,8 +1212,7 @@ static int register_test_probe(struct test_probe *probe)
>  {
>  	int ret;
>  
> -	if (probe->registered)
> -		BUG();
> +	BUG_ON(probe->registered);
>  
>  	ret = register_kprobe(&probe->kprobe);
>  	if (ret >= 0) {

[PATCH] ARM: EXYNOS: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/arm/mach-exynos/common.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 4e577f6..6a55a5a 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -465,10 +465,8 @@ static void __init combiner_cascade_irq(unsigned int combiner_nr, unsigned int i
 	else
 		max_nr = EXYNOS4_MAX_COMBINER_NR;
 
-	if (combiner_nr >= max_nr)
-		BUG();
-	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
-		BUG();
+	BUG_ON(combiner_nr >= max_nr);
+	BUG_ON(irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0);
 	irq_set_chained_handler(irq, combiner_handle_cascade_irq);
 }
 
-- 
1.7.10.4

Re: [PATCH] ARM: EXYNOS: use BUG_ON where possible

[Maarten Lankhorst]

Op 08-11-12 21:23, Sasha Levin schreef:
> Just use BUG_ON() instead of constructions such as:
>
> 	if (...)
> 		BUG()
>
> A simplified version of the semantic patch that makes this transformation
> is as follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression e;
> @@
> - if (e) BUG();
> + BUG_ON(e);
> // </smpl>
>
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  arch/arm/mach-exynos/common.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index 4e577f6..6a55a5a 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -465,10 +465,8 @@ static void __init combiner_cascade_irq(unsigned int combiner_nr, unsigned int i
>  	else
>  		max_nr = EXYNOS4_MAX_COMBINER_NR;
>  
> -	if (combiner_nr >= max_nr)
> -		BUG();
> -	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
> -		BUG();
> +	BUG_ON(combiner_nr >= max_nr);
> +	BUG_ON(irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0);
Is it really a good idea to put functions that perform work in a BUG_ON?
I don't know, but for some reason it just feels wrong. I'd expect code to
compile fine if BUG_ON was a noop, so doing verification calls only, not
actual work..

~Maarten

Re: [PATCH] ARM: EXYNOS: use BUG_ON where possible

[Russell King - ARM Linux]

On Mon, Nov 12, 2012 at 04:12:29PM +0100, Maarten Lankhorst wrote:
> Op 08-11-12 21:23, Sasha Levin schreef:
> > @@ -465,10 +465,8 @@ static void __init combiner_cascade_irq(unsigned int combiner_nr, unsigned int i
> >  	else
> >  		max_nr = EXYNOS4_MAX_COMBINER_NR;
> >  
> > -	if (combiner_nr >= max_nr)
> > -		BUG();
> > -	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
> > -		BUG();
> > +	BUG_ON(combiner_nr >= max_nr);
> > +	BUG_ON(irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0);
>
> Is it really a good idea to put functions that perform work in a BUG_ON?
> I don't know, but for some reason it just feels wrong. I'd expect code to
> compile fine if BUG_ON was a noop, so doing verification calls only, not
> actual work..

Well, it is currently defined as:

include/asm-generic/bug.h:#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
include/asm-generic/bug.h:#define BUG_ON(condition) do { if (condition) ; } while(0)

but as these can be overridden, I don't think relying on those
implementations is a good idea; to do so would be fragile.  Eg, what if
the BUG_ON() implementation becomes just:

#define BUG_ON(x)

then the function call itself vanishes.  So, only put the actual bug test
inside a BUG_ON(), not the functional part which must always be executed.

Re: [PATCH] ARM: EXYNOS: use BUG_ON where possible

[Sasha Levin]

On 11/12/2012 10:23 AM, Russell King - ARM Linux wrote:
> On Mon, Nov 12, 2012 at 04:12:29PM +0100, Maarten Lankhorst wrote:
>> Op 08-11-12 21:23, Sasha Levin schreef:
>>> @@ -465,10 +465,8 @@ static void __init combiner_cascade_irq(unsigned int combiner_nr, unsigned int i
>>>  	else
>>>  		max_nr = EXYNOS4_MAX_COMBINER_NR;
>>>  
>>> -	if (combiner_nr >= max_nr)
>>> -		BUG();
>>> -	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
>>> -		BUG();
>>> +	BUG_ON(combiner_nr >= max_nr);
>>> +	BUG_ON(irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0);
>>
>> Is it really a good idea to put functions that perform work in a BUG_ON?
>> I don't know, but for some reason it just feels wrong. I'd expect code to
>> compile fine if BUG_ON was a noop, so doing verification calls only, not
>> actual work..
> 
> Well, it is currently defined as:
> 
> include/asm-generic/bug.h:#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
> include/asm-generic/bug.h:#define BUG_ON(condition) do { if (condition) ; } while(0)
> 
> but as these can be overridden, I don't think relying on those
> implementations is a good idea; to do so would be fragile.  Eg, what if
> the BUG_ON() implementation becomes just:
> 
> #define BUG_ON(x)
> 
> then the function call itself vanishes.  So, only put the actual bug test
> inside a BUG_ON(), not the functional part which must always be executed.

Even if we ignore that modifying the side-effects is wrong, there's already
more than enough code in the kernel (both in kernel/ / mm/, and in arch/) to
cause breakage if for some reason the expression is not evaluated.

If some arch decides to not evaluate the expression there it's going to be
inherently broken.


Thanks,
Sasha

Re: [PATCH] ARM: EXYNOS: use BUG_ON where possible

[Sasha Levin]

On 11/12/2012 10:12 AM, Maarten Lankhorst wrote:
> Op 08-11-12 21:23, Sasha Levin schreef:
>> Just use BUG_ON() instead of constructions such as:
>>
>> 	if (...)
>> 		BUG()
>>
>> A simplified version of the semantic patch that makes this transformation
>> is as follows: (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @@
>> expression e;
>> @@
>> - if (e) BUG();
>> + BUG_ON(e);
>> // </smpl>
>>
>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
>> ---
>>  arch/arm/mach-exynos/common.c |    6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
>> index 4e577f6..6a55a5a 100644
>> --- a/arch/arm/mach-exynos/common.c
>> +++ b/arch/arm/mach-exynos/common.c
>> @@ -465,10 +465,8 @@ static void __init combiner_cascade_irq(unsigned int combiner_nr, unsigned int i
>>  	else
>>  		max_nr = EXYNOS4_MAX_COMBINER_NR;
>>  
>> -	if (combiner_nr >= max_nr)
>> -		BUG();
>> -	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
>> -		BUG();
>> +	BUG_ON(combiner_nr >= max_nr);
>> +	BUG_ON(irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0);
> Is it really a good idea to put functions that perform work in a BUG_ON?
> I don't know, but for some reason it just feels wrong. I'd expect code to
> compile fine if BUG_ON was a noop, so doing verification calls only, not
> actual work..

You can't modify the side-effects of a macro based on kernel configuration. If
we're evaluating the expression when BUG_ON() is enabled, you must also evaluate
the expression when BUG_ON() is not enabled (HAVE_ARCH_BUG_ON is not set).

The only reason I'd not include function calls in a BUG_ON() call is due to
readability considerations. In this case it looked okay to me, but if someone
thinks that getting the function call into the BUG_ON() complicated things I'm
fine with skipping that.


Thanks,
Sasha

[PATCH] ARM: integrator: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/arm/mach-integrator/pci_v3.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c
index bbeca59..85938de 100644
--- a/arch/arm/mach-integrator/pci_v3.c
+++ b/arch/arm/mach-integrator/pci_v3.c
@@ -191,12 +191,9 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
 	/*
 	 * Trap out illegal values
 	 */
-	if (offset > 255)
-		BUG();
-	if (busnr > 255)
-		BUG();
-	if (devfn > 255)
-		BUG();
+	BUG_ON(offset > 255);
+	BUG_ON(busnr > 255);
+	BUG_ON(devfn > 255);
 
 	if (busnr == 0) {
 		int slot = PCI_SLOT(devfn);
-- 
1.7.10.4

Re: [PATCH] ARM: integrator: use BUG_ON where possible

[Arnd Bergmann]

On Thursday 08 November 2012, Sasha Levin wrote:
> Just use BUG_ON() instead of constructions such as:
> 
> 	if (...)
> 		BUG()
> 
> A simplified version of the semantic patch that makes this transformation
> is as follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e;
> @@
> - if (e) BUG();
> + BUG_ON(e);
> // </smpl>
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Linus Walleij is doing most of the integrator work these days, maybe he
wants to apply the patch.

Acked-by: Arnd Bergmann <arnd@arndb.de>

>  arch/arm/mach-integrator/pci_v3.c |    9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c
> index bbeca59..85938de 100644
> --- a/arch/arm/mach-integrator/pci_v3.c
> +++ b/arch/arm/mach-integrator/pci_v3.c
> @@ -191,12 +191,9 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
>  	/*
>  	 * Trap out illegal values
>  	 */
> -	if (offset > 255)
> -		BUG();
> -	if (busnr > 255)
> -		BUG();
> -	if (devfn > 255)
> -		BUG();
> +	BUG_ON(offset > 255);
> +	BUG_ON(busnr > 255);
> +	BUG_ON(devfn > 255);
>  
>  	if (busnr == 0) {
>  		int slot = PCI_SLOT(devfn);
> -- 
> 1.7.10.4
> 
> 

Re: [PATCH] ARM: integrator: use BUG_ON where possible

[Linus Walleij]

On Mon, Nov 12, 2012 at 9:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:

>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
>
> Linus Walleij is doing most of the integrator work these days, maybe he
> wants to apply the patch.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

OK I'll apply it to my stash and submit a pull request to
simplify things.

I could split this into its own topic but with noone else changing
anything orthogonally on the Integrator I don't know if it's really
worth it, probably it's just better to take the stuff I sent yesterday
and a patch I made this evening and request it all to be pulled
at once.

Yours,
Linus Walleij

[PATCH] ARM: OMAP1: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/arm/mach-omap1/board-fsample.c  |    3 +--
 arch/arm/mach-omap1/board-h2.c       |    3 +--
 arch/arm/mach-omap1/board-h3.c       |    3 +--
 arch/arm/mach-omap1/board-perseus2.c |    3 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap1/board-fsample.c b/arch/arm/mach-omap1/board-fsample.c
index 8b5800a..7ca6cc4 100644
--- a/arch/arm/mach-omap1/board-fsample.c
+++ b/arch/arm/mach-omap1/board-fsample.c
@@ -307,8 +307,7 @@ static void __init omap_fsample_init(void)
 
 	fsample_init_smc91x();
 
-	if (gpio_request(FSAMPLE_NAND_RB_GPIO_PIN, "NAND ready") < 0)
-		BUG();
+	BUG_ON(gpio_request(FSAMPLE_NAND_RB_GPIO_PIN, "NAND ready") < 0);
 	gpio_direction_input(FSAMPLE_NAND_RB_GPIO_PIN);
 
 	omap_cfg_reg(L3_1610_FLASH_CS2B_OE);
diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
index 9134b64..4953cf7 100644
--- a/arch/arm/mach-omap1/board-h2.c
+++ b/arch/arm/mach-omap1/board-h2.c
@@ -412,8 +412,7 @@ static void __init h2_init(void)
 
 	h2_nand_resource.end = h2_nand_resource.start = OMAP_CS2B_PHYS;
 	h2_nand_resource.end += SZ_4K - 1;
-	if (gpio_request(H2_NAND_RB_GPIO_PIN, "NAND ready") < 0)
-		BUG();
+	BUG_ON(gpio_request(H2_NAND_RB_GPIO_PIN, "NAND ready") < 0);
 	gpio_direction_input(H2_NAND_RB_GPIO_PIN);
 
 	omap_cfg_reg(L3_1610_FLASH_CS2B_OE);
diff --git a/arch/arm/mach-omap1/board-h3.c b/arch/arm/mach-omap1/board-h3.c
index bf213d1..563ba16 100644
--- a/arch/arm/mach-omap1/board-h3.c
+++ b/arch/arm/mach-omap1/board-h3.c
@@ -406,8 +406,7 @@ static void __init h3_init(void)
 
 	nand_resource.end = nand_resource.start = OMAP_CS2B_PHYS;
 	nand_resource.end += SZ_4K - 1;
-	if (gpio_request(H3_NAND_RB_GPIO_PIN, "NAND ready") < 0)
-		BUG();
+	BUG_ON(gpio_request(H3_NAND_RB_GPIO_PIN, "NAND ready") < 0);
 	gpio_direction_input(H3_NAND_RB_GPIO_PIN);
 
 	/* GPIO10 Func_MUX_CTRL reg bit 29:27, Configure V2 to mode1 as GPIO */
diff --git a/arch/arm/mach-omap1/board-perseus2.c b/arch/arm/mach-omap1/board-perseus2.c
index 030bd48..67c2612 100644
--- a/arch/arm/mach-omap1/board-perseus2.c
+++ b/arch/arm/mach-omap1/board-perseus2.c
@@ -275,8 +275,7 @@ static void __init omap_perseus2_init(void)
 
 	perseus2_init_smc91x();
 
-	if (gpio_request(P2_NAND_RB_GPIO_PIN, "NAND ready") < 0)
-		BUG();
+	BUG_ON(gpio_request(P2_NAND_RB_GPIO_PIN, "NAND ready") < 0);
 	gpio_direction_input(P2_NAND_RB_GPIO_PIN);
 
 	omap_cfg_reg(L3_1610_FLASH_CS2B_OE);
-- 
1.7.10.4

Re: [PATCH] ARM: OMAP1: use BUG_ON where possible

[Tony Lindgren]

* Sasha Levin <sasha.levin@oracle.com> [121108 12:26]:
> Just use BUG_ON() instead of constructions such as:
> 
> 	if (...)
> 		BUG()
> 
> A simplified version of the semantic patch that makes this transformation
> is as follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression e;
> @@
> - if (e) BUG();
> + BUG_ON(e);
> // </smpl>
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Thanks applying into omap-for-v3.8/board-v2.

Regards,

Tony

[PATCH] ARM: dma: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/arm/mach-rpc/dma.c     |    3 +--
 arch/arm/mach-s3c64xx/dma.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-rpc/dma.c b/arch/arm/mach-rpc/dma.c
index 85883b2..92e22ba 100644
--- a/arch/arm/mach-rpc/dma.c
+++ b/arch/arm/mach-rpc/dma.c
@@ -265,8 +265,7 @@ static void floppy_enable_dma(unsigned int chan, dma_t *dma)
 	unsigned int fiqhandler_length;
 	struct pt_regs regs;
 
-	if (fdma->dma.sg)
-		BUG();
+	BUG_ON(fdma->dma.sg);
 
 	if (fdma->dma.dma_mode == DMA_MODE_READ) {
 		extern unsigned char floppy_fiqin_start, floppy_fiqin_end;
diff --git a/arch/arm/mach-s3c64xx/dma.c b/arch/arm/mach-s3c64xx/dma.c
index f2a7a17..585c2ae 100644
--- a/arch/arm/mach-s3c64xx/dma.c
+++ b/arch/arm/mach-s3c64xx/dma.c
@@ -603,8 +603,7 @@ static irqreturn_t s3c64xx_dma_irq(int irq, void *pw)
 				&& buff->next != chan->next)
 			buff = buff->next;
 
-		if (!buff)
-			BUG();
+		BUG_ON(!buff);
 
 		if (buff == chan->next)
 			buff = chan->end;
-- 
1.7.10.4

[PATCH] ARM: versatile: use BUG_ON where possible

[Sasha Levin]

Just use BUG_ON() instead of constructions such as:

	if (...)
		BUG()

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@
- if (e) BUG();
+ BUG_ON(e);
// </smpl>

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/arm/mach-versatile/pci.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-versatile/pci.c b/arch/arm/mach-versatile/pci.c
index 2f84f40..3936a11 100644
--- a/arch/arm/mach-versatile/pci.c
+++ b/arch/arm/mach-versatile/pci.c
@@ -82,12 +82,9 @@ static void __iomem *__pci_addr(struct pci_bus *bus,
 	/*
 	 * Trap out illegal values
 	 */
-	if (offset > 255)
-		BUG();
-	if (busnr > 255)
-		BUG();
-	if (devfn > 255)
-		BUG();
+	BUG_ON(offset > 255);
+	BUG_ON(busnr > 255);
+	BUG_ON(devfn > 255);
 
 	return VERSATILE_PCI_CFG_VIRT_BASE + ((busnr << 16) |
 		(PCI_SLOT(devfn) << 11) | (PCI_FUNC(devfn) << 8) | offset);
-- 
1.7.10.4

Re: [PATCH] vmxnet3: convert BUG_ON(true) into a simple BUG()

[Shreyas Bhatewara]

----- Original Message -----
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  drivers/net/vmxnet3/vmxnet3_drv.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Signed-off-by: Shreyas N Bhatewara <sbhatewara@vmware.com>

Re: [PATCH] vmxnet3: convert BUG_ON(true) into a simple BUG()

[David Miller]

From: Shreyas Bhatewara <sbhatewara@vmware.com>
Date: Thu, 8 Nov 2012 22:02:16 -0800 (PST)

> 
> ----- Original Message -----
>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
>> ---
>>  drivers/net/vmxnet3/vmxnet3_drv.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Signed-off-by: Shreyas N Bhatewara <sbhatewara@vmware.com>

Applied, thanks.

Re: [PATCH] vmxnet3: convert BUG_ON(true) into a simple BUG()

[Ryan Mallon]

On 09/11/12 07:23, Sasha Levin wrote:
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  drivers/net/vmxnet3/vmxnet3_drv.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index 0ae1bcc..7e9622f 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -1922,7 +1922,7 @@ vmxnet3_free_irqs(struct vmxnet3_adapter *adapter)
>  		free_irq(adapter->pdev->irq, adapter->netdev);
>  		break;
>  	default:
> -		BUG_ON(true);
> +		BUG();
>  	}
>  }

All of the BUG_ON tests in this function look like programming error
assertions. It looks like the worst that would happen is that some irqs
might not be properly released, not the sort of thing that is going to
make the system unstable if you don't bug.

Can't they just be replaced with (for example):

	if (WARN_ON(blah))
		return;

Or even just:

	netdev_err(adapter->netdev, "bad irq type %d for free\n",
			intr->type);

~Ryan