[PATCH] Remove useless assertions from reiserfs

[PATCH] Remove useless assertions from reiserfs

[davej]

diff -urpN --exclude-from=/home/davej/.exclude bk-linus/fs/reiserfs/hashes.c linux-2.5/fs/reiserfs/hashes.c
--- bk-linus/fs/reiserfs/hashes.c	2003-04-10 06:01:29.000000000 +0100
+++ linux-2.5/fs/reiserfs/hashes.c	2003-07-13 06:04:57.000000000 +0100
@@ -90,10 +90,6 @@ u32 keyed_hash(const signed char *msg, i
 
 	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 @@ u32 keyed_hash(const signed char *msg, i
 	}
 	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 @@ u32 keyed_hash(const signed char *msg, i
 	}
 	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 @@ u32 keyed_hash(const signed char *msg, i
 	}
 	else
 	{
-	    	//assert(len < 4);
-		if (len >= 4)
-		    BUG();
 		a = b = c = d = pad;
 		for(i = 0; i < len; i++)
 		{

Re: [PATCH] Remove useless assertions from reiserfs

[Jeff Garzik]

Why are these useless?

davej@redhat.com wrote:
> diff -urpN --exclude-from=/home/davej/.exclude bk-linus/fs/reiserfs/hashes.c linux-2.5/fs/reiserfs/hashes.c
> --- bk-linus/fs/reiserfs/hashes.c	2003-04-10 06:01:29.000000000 +0100
> +++ linux-2.5/fs/reiserfs/hashes.c	2003-07-13 06:04:57.000000000 +0100
> @@ -90,10 +90,6 @@ u32 keyed_hash(const signed char *msg, i
>  
>  	if (len >= 12)
>  	{
> -	    	//assert(len < 16);
> -		if (len >= 16)
> -		    BUG();
> -
>  		a = (u32)msg[ 0]      |
>  		    (u32)msg[ 1] << 8 |
>  		    (u32)msg[ 2] << 16|

Seems like a valid check to me...

	Jeff

Re: [PATCH] Remove useless assertions from reiserfs

[Dave Jones]

On Mon, Aug 11, 2003 at 01:45:30PM -0400, Jeff Garzik wrote:
 > Why are these useless?

read the code not the diff.

 > >@@ -90,10 +90,6 @@ u32 keyed_hash(const signed char *msg, i
 > > 
 > > 	if (len >= 12)
 > > 	{
 > >-	    	//assert(len < 16);
 > >-		if (len >= 16)
 > >-		    BUG();
 > >-
 > > 		a = (u32)msg[ 0]      |
 > > 		    (u32)msg[ 1] << 8 |
 > > 		    (u32)msg[ 2] << 16|
 > 
 > Seems like a valid check to me...

Above this loop is another loop which we don't exit until len < 16

	Dave

-- 
 Dave Jones     http://www.codemonkey.org.uk

Re: [PATCH] Remove useless assertions from reiserfs

[Jeff Garzik]

Dave Jones wrote:
> On Mon, Aug 11, 2003 at 01:45:30PM -0400, Jeff Garzik wrote:
>  > Why are these useless?
> 
> read the code not the diff.
> 
>  > >@@ -90,10 +90,6 @@ u32 keyed_hash(const signed char *msg, i
>  > > 
>  > > 	if (len >= 12)
>  > > 	{
>  > >-	    	//assert(len < 16);
>  > >-		if (len >= 16)
>  > >-		    BUG();
>  > >-
>  > > 		a = (u32)msg[ 0]      |
>  > > 		    (u32)msg[ 1] << 8 |
>  > > 		    (u32)msg[ 2] << 16|
>  > 
>  > Seems like a valid check to me...
> 
> Above this loop is another loop which we don't exit until len < 16

ok, agreed

	Jeff

Re: [PATCH] Remove useless assertions from reiserfs

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

On Mon, 11 Aug 2003 13:45:30 EDT, Jeff Garzik said:
> Why are these useless?

> >  	if (len >= 12)
> >  	{          
> >      //assert(len < 16);
> >               if (len >= 16)
> >                    BUG();
> 
> Seems like a valid check to me...

Just before that, there's code:

     while(len >= 16)
        {
	...
                len -= 16;
            }

So if that ever exits with a len >=16, we have a SERIOUS problem with
either the compiler or the hardware - as such, that "if (..) BUG" is dead code.
Similarly for the other checks.

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] Remove useless assertions from reiserfs

[Petr Vandrovec]

On 11 Aug 03 at 14:00, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 11 Aug 2003 13:45:30 EDT, Jeff Garzik said:
> > Why are these useless?
> 
> > >     if (len >= 12)
> > >     {          
> > >      //assert(len < 16);
> > >               if (len >= 16)
> > >                    BUG();
> > 
> > Seems like a valid check to me...
> 
> Just before that, there's code:
> 
>      while(len >= 16)
>         {
>     ...
>                 len -= 16;
>             }
> 
> So if that ever exits with a len >=16, we have a SERIOUS problem with
> either the compiler or the hardware - as such, that "if (..) BUG" is dead code.
> Similarly for the other checks.

I always thought that assertions are just for that - if you can hit them
without some unexpected event/bug, you have a SERIOUS problem.
                                                    Petr Vandrovec
                                                    

Re: [PATCH] Remove useless assertions from reiserfs

[Dave Jones]

On Mon, Aug 11, 2003 at 08:33:13PM +0200, Petr Vandrovec wrote:

 > I always thought that assertions are just for that - if you can hit them
 > without some unexpected event/bug, you have a SERIOUS problem.

Not for when you explicitly code things above so it really cannot
happen. An assertion is more for the case "I really hope someone
doesn't pass me x in state y" type assertions than "I really hope
the compiler did the right thing with the previous loop"

		Dave

-- 
 Dave Jones     http://www.codemonkey.org.uk