LinuxLists.cc - [PATCH] vmxnet3: convert BUG

2012-11-08 20:24:24

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

Signed-off-by: Sasha Levin <[email protected]>
---
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

2012-11-08 20:24:46

by Sasha Levin

[permalink] [raw]

Subject: [PATCH] ARM: gic: use BUG_ON where possible

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 <[email protected]>
---
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

2012-11-08 20:24:51

by Sasha Levin

[permalink] [raw]

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

2012-11-08 20:25:04

by Sasha Levin

[permalink] [raw]

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

2012-11-08 20:25:15

by Sasha Levin

[permalink] [raw]

Subject: [PATCH] ARM: dma: use BUG_ON where possible

2012-11-08 20:25:01

by Sasha Levin

[permalink] [raw]

Subject: [PATCH] ARM: versatile: use BUG_ON where possible

2012-11-08 20:25:56

by Sasha Levin

[permalink] [raw]

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

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 <[email protected]>
---
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

2012-11-08 20:26:18

by Sasha Levin

[permalink] [raw]

Subject: [PATCH] alpha: use BUG_ON where possible

2012-11-08 20:24:45

by Sasha Levin

[permalink] [raw]

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

2012-11-08 21:42:54

by Jiri Kosina

[permalink] [raw]

Subject: Re: [PATCH] alpha: use BUG_ON where possible

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

2012-11-09 06:02:20

by Shreyas Bhatewara

[permalink] [raw]

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

----- Original Message -----
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> drivers/net/vmxnet3/vmxnet3_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Signed-off-by: Shreyas N Bhatewara <[email protected]>

2012-11-09 09:26:31

by Jon Medhurst (Tixy)

[permalink] [raw]

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

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 <[email protected]>

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 <[email protected]>

> ---
> 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) {

2012-11-09 16:23:55

by Sasha Levin

[permalink] [raw]

Subject: Re: [PATCH] alpha: use BUG_ON where possible

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

2012-11-09 22:03:31

by David Miller

[permalink] [raw]

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

From: Shreyas Bhatewara <[email protected]>
Date: Thu, 8 Nov 2012 22:02:16 -0800 (PST)

>
> ----- Original Message -----
>> Signed-off-by: Sasha Levin <[email protected]>
>> ---
>> drivers/net/vmxnet3/vmxnet3_drv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Signed-off-by: Shreyas N Bhatewara <[email protected]>

Applied, thanks.

2012-11-11 22:27:46

by Ryan Mallon

[permalink] [raw]

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

On 09/11/12 07:23, Sasha Levin wrote:
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> 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

2012-11-12 14:43:35

by Jiri Kosina

[permalink] [raw]

Subject: Re: [PATCH] alpha: use BUG_ON where possible

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

2012-11-12 15:03:29

by Sasha Levin

[permalink] [raw]

Subject: Re: [PATCH] alpha: use BUG_ON where possible

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

2012-11-12 15:12:37

by Maarten Lankhorst

[permalink] [raw]

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

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 <[email protected]>
> ---
> 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

2012-11-12 15:25:05

by Russell King - ARM Linux

[permalink] [raw]

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

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.

2012-11-12 15:26:49

by Sasha Levin

[permalink] [raw]

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

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 <[email protected]>
>> ---
>> 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

2012-11-12 15:53:08

by Sasha Levin

[permalink] [raw]

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

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

2012-11-12 20:44:21

by Arnd Bergmann

[permalink] [raw]

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

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 <[email protected]>

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

Acked-by: Arnd Bergmann <[email protected]>

> 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
>
>

2012-11-12 23:21:25

by Tony Lindgren

[permalink] [raw]

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

* Sasha Levin <[email protected]> [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 <[email protected]>

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

Regards,

Tony

2012-11-17 18:41:59

by Linus Walleij

[permalink] [raw]

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

On Mon, Nov 12, 2012 at 9:44 PM, Arnd Bergmann <[email protected]> wrote:

>> Signed-off-by: Sasha Levin <[email protected]>
>
> Linus Walleij is doing most of the integrator work these days, maybe he
> wants to apply the patch.
>
> Acked-by: Arnd Bergmann <[email protected]>

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