Re: fs/minix/bitmap.c is broken in CVS

[mjn3@codepoet.org (Manuel Novoa III)]

Hello Riley,

On Thu, Jun 20, 2002 at 06:45:03PM +0100, Riley Williams wrote:
> Hi Manuel.
> 
> > Apparently during the recent changes, someone replaced the code
> > sequence
> >
> >    if (j == find_first_zero_bit(bh->b_data, 8192))
> >	panic("still zero bit!%d\n", j);
> >
> > with
> >
> >    j == (block_t) find_first_zero_bit(bh->b_data, 8192);
> >    if (j)
> >	panic("still zero bit!%d\n", j);
> >
> > in function minix_new_block in file fs/minix/bitmap.c.
> 
> If they did, it wouldn't compile. 

Sure it would compile.  A better compiler than bcc would probably issue
a warning because the result of the == operator would be discarded, but
it is still valid C.

>           However, there is only one = sign
> (it's the assignment operator, not the equality comparison operator
> that occurs there).

Riley, look at the code.  It is _supposed_ to be == since the intent
is to check and make sure the bit you want set by set_bit has actually
been set.  I found this because trying to write to a new file was
causing a kernel panic.

> I also replaced similar sequences elsewhere in the kernel source.

Unfortunatly, doing so can be an anti-optimization wrt size when
using bcc.  My size reduction patch probably reversed a number of
your changes.

> I've been running the ELKS kernel source through the splint source
> checking tool, and cleaning the source up so that splint doesn't report
> any warnings whatsoever in its -weak analysis mode. Here's the version
> of those two lines that reports clean as far as splint is concerned...
> 
> >    if ((j = ((block_t) find_first_zero_bit(bh->b_data, 8192))))
> >	panic("still zero bit!%d\n", j);

If you check the older versions in cvs, you'll see that it was == for
comparison, not = for assignment.

> I don't know about you, but there's far too many brackets in there for
> my liking, which is why I split it into two lines. It doesn't affect the
> code produced.

The parenthesis don't bother me as long as the code is formatted
reasonably. And there are situations were it _does_ affect the code
produced.  If you'd like, I'd be happy to provide you with some
examples.  That would have to wait until Monday or so as I am currently
on the road.

> Best wishes from Riley.

And to you,

Manuel