2002-01-06 12:31:02

by mumismo

[permalink] [raw]
Subject: [PATCH]: 2.5.1pre9 change several if (x) BUG to BUG_ON(x)



Yes, only that, even a trained monkey is able to make this patch, but i think
is a good way to make people confortable with BUG_ON
I have changed all the BUG to BUG_ON possible except the directories arch
include, net, fs and drivers.
If you apply this patch i'll change that directories at once.
I have no really change anything so it's totally sane apply the patch.



Attachments:
BUG_ON.patch (32.04 kB)
make BUG_ON common in the standard kernel

2002-01-06 14:37:23

by Alan

[permalink] [raw]
Subject: Re: [PATCH]: 2.5.1pre9 change several if (x) BUG to BUG_ON(x)

> Yes, only that, even a trained monkey is able to make this patch, but i think
> is a good way to make people confortable with BUG_ON

Your patch looks wrong (ook ook! 8)) - if you build without BUG enabled you
don't make various function calls with your change. BUG_ON has the C nasty
assert() does that makes it a horrible horrible idea and its unfortunate
it got put in.

BUG_ON(function(x,y))

ends up not causing function to be called when not debugging

The classic C mess people get into is similar with things like

assert(x++ == 4);

2002-01-06 14:49:03

by Momchil Velikov

[permalink] [raw]
Subject: Re: [PATCH]: 2.5.1pre9 change several if (x) BUG to BUG_ON(x)

>>>>> "Alan" == Alan Cox <[email protected]> writes:

>> Yes, only that, even a trained monkey is able to make this patch, but i think
>> is a good way to make people confortable with BUG_ON

Alan> Your patch looks wrong (ook ook! 8)) - if you build without BUG enabled you
Alan> don't make various function calls with your change. BUG_ON has the C nasty
Alan> assert() does that makes it a horrible horrible idea and its unfortunate
Alan> it got put in.

Alan> BUG_ON(function(x,y))

#ifdef DEBUG
#define BUG_ON(x) if (x) BUG()
#else
#define BUG_ON(x) (void)(x)
#endif

2002-01-06 16:06:25

by mumismo

[permalink] [raw]
Subject: Re: [PATCH]: 2.5.1pre9 change several if (x) BUG to BUG_ON(x)

On Sunday 06 January 2002 15:48, Momchil Velikov wrote:

>
> Alan> Your patch looks wrong (ook ook! 8)) - if you build without BUG
> enabled you Alan> don't make various function calls with your change.
> BUG_ON has the C nasty Alan> assert() does that makes it a horrible
> horrible idea and its unfortunate Alan> it got put in.
>
> Alan> BUG_ON(function(x,y))
>
> #ifdef DEBUG
> #define BUG_ON(x) if (x) BUG()
> #else
> #define BUG_ON(x) (void)(x)
> #endif

Sorry but in 2.5.2 pre9 (i make a mistake in the topic, is 2.5.2pre9 , no
2.5.1pre9):
kernel.h:
#define BUG_ON(condition) do { if (unlikely((condition)!=0)) BUG(); } while(0)

So i don't understand Alan's complaims, if i understands it well, BUG_ON() is
just a optimization of BUG function.
So in simple "if (condition) BUG()" cases i think it will allways be replaced
, of course in "if (condition) someWork; BUG()" we can't.

2002-01-06 16:15:45

by Alan

[permalink] [raw]
Subject: Re: [PATCH]: 2.5.1pre9 change several if (x) BUG to BUG_ON(x)

> Alan> BUG_ON(function(x,y))
>
> #ifdef DEBUG
> #define BUG_ON(x) if (x) BUG()
> #else
> #define BUG_ON(x) (void)(x)
> #endif

(void)(x) may cause x not be evaluated if the compiler can optimise it out
on the grounds that the result is discarded. Fortunately gcc can't remove
anything that has side effects so providing there are no other bugs in the
code (eg referencing mmio addresses) it should be fine - you are correct