Skip to contentSkip to conversation

What policy for BUG_ON()?

DISCUSSION Adrian Bunk 5 participants 6 messages Aug 30, 2004 – Aug 31, 2004

lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

Let me try to summarize the different options regarding BUG_ON, concerning whether the argument to BUG_ON might contain side effects, and whether it should be allowed in some "do this only if you _really_ know what you are doing" situations to let BUG_ON do nothing.

Options:
1. BUG_ON must not be defined to do nothing
1a. side effects are allowed in the argument of BUG_ON 1b. side effects are not allowed in the argument of BUG_ON
2. BUG_ON is allowed to be defined to do nothing 2a. side effects are allowed in the argument of BUG_ON 2b. side effects are not allowed in the argument of BUG_ON

It would be good if there was a decision which of the four choices should become documented policy.

<-- snip -->

My personal opinions:

IMHO, 1b doesn't make much sense, since in the case of 1. side effects are never a problem.

IMHO, 2b is bad since it might cause nasty heisenbugs if BUG_ON does nothing, and preserving the side effects is easy.

<-- snip -->

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#1 lore raw
Quoted 5 lines · Adrian Bunk

Let me try to summarize the different options regarding BUG_ON, concerning whether the argument to BUG_ON might contain side effects, and whether it should be allowed in some "do this only if you _really_ know what you are doing" situations to let BUG_ON do nothing.

Quoted 7 lines

Options:
1. BUG_ON must not be defined to do nothing
1a. side effects are allowed in the argument of BUG_ON 1b. side effects are not allowed in the argument of BUG_ON
2. BUG_ON is allowed to be defined to do nothing 2a. side effects are allowed in the argument of BUG_ON 2b. side effects are not allowed in the argument of BUG_ON

since you quoted me earlier my 2 cents:
1) I would prefer BUG_ON() arguments to not have side effects; its just cleaner that way. (similar to assert)

2) if one wants to compiel out BUG_ON, I rather alias it to panic() than to nothing.

#2 lore raw
Quoted 14 lines · Arjan van de Ven
Quoted 5 lines · Adrian Bunk

Let me try to summarize the different options regarding BUG_ON, concerning whether the argument to BUG_ON might contain side effects, and whether it should be allowed in some "do this only if you _really_ know what you are doing" situations to let BUG_ON do nothing.

Quoted 7 lines

Options:
1. BUG_ON must not be defined to do nothing
1a. side effects are allowed in the argument of BUG_ON 1b. side effects are not allowed in the argument of BUG_ON
2. BUG_ON is allowed to be defined to do nothing 2a. side effects are allowed in the argument of BUG_ON 2b. side effects are not allowed in the argument of BUG_ON

Quoted 6 lines

since you quoted me earlier my 2 cents:
1) I would prefer BUG_ON() arguments to not have side effects; its just cleaner that way. (similar to assert)

2) if one wants to compiel out BUG_ON, I rather alias it to panic() than to nothing.

I agree completely with that.

Signature · 1 line
Jens Axboe
#3 lore raw
Quoted 14 lines · Jens Axboe
Quoted 12 lines · Arjan van de Ven
Quoted 9 lines · Adrian Bunk

Let me try to summarize the different options regarding BUG_ON, concerning whether the argument to BUG_ON might contain side effects, and whether it should be allowed in some "do this only if you _really_ know what you are doing" situations to let BUG_ON do nothing.

Options:
1. BUG_ON must not be defined to do nothing
1a. side effects are allowed in the argument of BUG_ON 1b. side effects are not allowed in the argument of BUG_ON
2. BUG_ON is allowed to be defined to do nothing 2a. side effects are allowed in the argument of BUG_ON 2b. side effects are not allowed in the argument of BUG_ON

Quoted 8 lines
Quoted 5 lines

since you quoted me earlier my 2 cents:
1) I would prefer BUG_ON() arguments to not have side effects; its just cleaner that way. (similar to assert)

2) if one wants to compiel out BUG_ON, I rather alias it to panic() than to nothing.

I agree completely with that.

This would mean that the condition would still have to be tested which kind of defeats the purpose of removing the BUG_ON in the first place, doesn't it?

Signature · 4 lines
Paulo Marques - www.grupopie.com

To err is human, but to really foul things up requires a computer.
Farmers' Almanac, 1978
#4 lore raw
In reply to Adrian Bunk
Quoted 14 lines · Adrian Bunk

Let me try to summarize the different options regarding BUG_ON, concerning whether the argument to BUG_ON might contain side effects, and whether it should be allowed in some "do this only if you _really_ know what you are doing" situations to let BUG_ON do nothing.

Options:
1. BUG_ON must not be defined to do nothing
1a. side effects are allowed in the argument of BUG_ON 1b. side effects are not allowed in the argument of BUG_ON
2. BUG_ON is allowed to be defined to do nothing 2a. side effects are allowed in the argument of BUG_ON 2b. side effects are not allowed in the argument of BUG_ON

Quoted 2 lines

It would be good if there was a decision which of the four choices should become documented policy.

I'd suggest we strongly discourage side-effects in BUG_ON().

That said, it might be safest to just go for 1b - we make side-effects of BUG_ON() be _documented_ as a bug, but just for safety, I'd suggest doing

	#define BUG_ON(x) (void)(x)

anyway, if somebody wants to compile without debugging. That will still make the side-effects happen if somebody has them (and if there are none, the compiler will not generate any code anyway).

In other words: nobody should _depend_ on the side effects, and we should flame people who have them. A quick grep shows this:

	mm/slab.c:      BUG_ON(spin_trylock(&cachep->spinlock));

which makes no real sense - it looks like slab really wants to use "spin_is_locked()".

I could make some sparse extension that warns about side effects. I already calculate the "side-effectiveness" of an expression for other reasons..

Linus

author
#5 lore raw
Quoted 25 lines · Linus Torvalds
Quoted 14 lines · Adrian Bunk

Let me try to summarize the different options regarding BUG_ON, concerning whether the argument to BUG_ON might contain side effects, and whether it should be allowed in some "do this only if you _really_ know what you are doing" situations to let BUG_ON do nothing.

Options:
1. BUG_ON must not be defined to do nothing
1a. side effects are allowed in the argument of BUG_ON 1b. side effects are not allowed in the argument of BUG_ON
2. BUG_ON is allowed to be defined to do nothing 2a. side effects are allowed in the argument of BUG_ON 2b. side effects are not allowed in the argument of BUG_ON

Quoted 2 lines

It would be good if there was a decision which of the four choices should become documented policy.

I'd suggest we strongly discourage side-effects in BUG_ON().

That said, it might be safest to just go for 1b - we make side-effects of BUG_ON() be _documented_ as a bug, but just for safety, I'd suggest doing

Quoted 6 lines
	#define BUG_ON(x) (void)(x)

anyway, if somebody wants to compile without debugging. That will still make the side-effects happen if somebody has them (and if there are none, the compiler will not generate any code anyway).
...

You say 1b but describe 2b...

The difference between 1b and 2b is that a patch to
#define BUG_ON(x) (void)(x)
with an own option under EMBEDDED might be accepted into the kernel with 2b, but not with 1b.

Quoted 1 line

Linus

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed