2003-08-11 16:49:37

by Dave Jones

[permalink] [raw]
Subject: [PATCH] Remove useless assertions from reiserfs

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


2003-08-11 17:49:09

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] Remove useless assertions from reiserfs

Why are these useless?

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




2003-08-11 17:58:42

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] Remove useless assertions from reiserfs

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

2003-08-11 17:59:16

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] Remove useless assertions from reiserfs

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




2003-08-11 18:18:01

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH] Remove useless assertions from reiserfs

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.


Attachments:
(No filename) (226.00 B)

2003-08-11 18:44:31

by Petr Vandrovec

[permalink] [raw]
Subject: Re: [PATCH] Remove useless assertions from reiserfs

On 11 Aug 03 at 14:00, [email protected] 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


2003-08-11 18:48:10

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] Remove useless assertions from reiserfs

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