Skip to contentSkip to conversation

BUGed to death

DISCUSSION [email protected] 5 participants 6 messages Apr 15, 2003

lore
Share & tools
lore

[email protected]

author
#0 lore raw

The patch below eliminates 4 BUG() calls that clearly cannot happen based on the context.

fs/reiserfs/hashes.c renamed from fs/reiserfs/hashes.c.orig +0 -13
@@ -90,10 +90,6 @@
 
 	if (len >= 12)
 	{
-	    	//assert(len < 16);
-		if (len >= 16)
-		    BUG();
-
 		a = (u32)msg[ 0]      |
 		    (u32)msg[ 1] << 8 |
 		    (u32)msg[ 2] << 16|

@@ -116,9 +112,6 @@
 	}
 	else if (len >= 8)
 	{
-	    	//assert(len < 12);
-		if (len >= 12)
-		    BUG();
 		a = (u32)msg[ 0]      |
 		    (u32)msg[ 1] << 8 |
 		    (u32)msg[ 2] << 16|

@@ -137,9 +130,6 @@
 	}
 	else if (len >= 4)
 	{
-	    	//assert(len < 8);
-		if (len >= 8)
-		    BUG();
 		a = (u32)msg[ 0]      |
 		    (u32)msg[ 1] << 8 |
 		    (u32)msg[ 2] << 16|

@@ -154,9 +144,6 @@
 	}
 	else
 	{
-	    	//assert(len < 4);
-		if (len >= 4)
-		    BUG();
 		a = b = c = d = pad;
 		for(i = 0; i < len; i++)
 		{
Signature · 2 lines
Randy Hron
http://home.earthlink.net/~rwhron/kernel/bigbox.html
#1 lore raw
Quoted 2 lines · On Tue, Apr 15, 2003 at 10:30:24AM -0400, [email protected] wrote:

The patch below eliminates 4 BUG() calls that clearly cannot happen based on the context.

This looks bogus.

fs/reiserfs/hashes.c line 96

Quoted 10 lines
fs/reiserfs/hashes.c renamed from fs/reiserfs/hashes.c.orig +0 -4
@@ -90,10 +90,6 @@
 
 	if (len >= 12)
 	{
-	    	//assert(len < 16);
-		if (len >= 16)
-		    BUG();
-

Imagine I pass in 20. Previously, the BUG triggers. Not any more. Ditto the other changes. Or am _I_ missing something ?

Dave

#2 lore raw
Quoted 9 lines · Dave Jones
Quoted 1 line · On Tue, Apr 15, 2003 at 10:30:24AM -0400, [email protected] wrote:

The patch below eliminates 4 BUG() calls that clearly cannot happen based on the context.

This looks bogus.

Quoted 4 lines
fs/reiserfs/hashes.c renamed from fs/reiserfs/hashes.c.orig +0 -0
@@ -90,10 +90,6 @@
Quoted 10 lines

fs/reiserfs/hashes.c line 96

Quoted 6 lines
fs/reiserfs/hashes.c renamed from fs/reiserfs/hashes.c.orig +0 -4
 	if (len >= 12)
 	{
-	    	//assert(len < 16);
-		if (len >= 16)
-		    BUG();
-

Imagine I pass in 20. Previously, the BUG triggers. Not any more. Ditto the other changes. Or am _I_ missing something ?

Just from the context of the patch, you are right with the first one. Subsequent bugs can be removed due to their possibility being eliminated by previous if statements. The code suggests that len will trivially not be >= 16 at this point, however. So the patch is ok.

#3 lore raw
In reply to Dave Jones
Quoted 3 lines · Dave Jones
Quoted 2 lines · On Tue, Apr 15, 2003 at 10:30:24AM -0400, [email protected] wrote:

The patch below eliminates 4 BUG() calls that clearly cannot happen based on the context.

[...]

Quoted 2 lines

Imagine I pass in 20. Previously, the BUG triggers. Not any more. Ditto the other changes. Or am _I_ missing something ?

Am I having a deja vu ? For me it seems this patch has been posted some time ago and I was also fooled by the patch at first.

BUT: the original author is right. You just have to look at the original code not only the patch sniplet.

Soeren.

#4 lore raw
In reply to Dave Jones
Quoted 2 lines · Dave Jones

Imagine I pass in 20. Previously, the BUG triggers. Not any more. Ditto the other changes. Or am _I_ missing something ?

Yes ;)
You're missing the (not posted) while loop before. Just look into source.

Signature · 5 lines
Regards Michael Buesch.
http://www.8ung.at/tuxsoft

$ cat /dev/zero > /dev/null
/dev/null: That's *not* funny! :(
#5 lore raw
Quoted 6 lines · Michael Buesch
Quoted 2 lines · Dave Jones

Imagine I pass in 20. Previously, the BUG triggers. Not any more. Ditto the other changes. Or am _I_ missing something ?

Yes ;)
You're missing the (not posted) while loop before. Just look into source.

Yup. I stand corrected.

Dave