Re: [PATCH] fixes for linked list bugs in block I/O code

[Dave Peterson <dsp@llnl.gov>]

On Wednesday 07 May 2003 05:26 pm, Bartlomiej Zolnierkiewicz wrote:
> On Wed, 7 May 2003, Dave Peterson wrote:
> > On Wednesday 07 May 2003 04:42 pm, Bartlomiej Zolnierkiewicz wrote:
> > > > ========== START OF 2.5.69 PATCH FOR drivers/block/ll_rw_blk.c
> > > > =========== --- ll_rw_blk.c.old     Wed May  7 15:55:18 2003
> > > > +++ ll_rw_blk.c.new     Wed May  7 16:01:56 2003
> > > > @@ -1721,6 +1721,7 @@
> > > >                                 break;
> > > >                         }
> > > >
> > > > +                       bio->bi_next = req->biotail->bi_next;
> > >
> > > This is simply wrong, look at the line below.
> > >
> > > >                         req->biotail->bi_next = bio;
> > >
> > > req->bio - first bio
> > > req->bio->bi_next - next bio
> > > ...
> > > req->biotail - last bio
> > >
> > > so req->biotail->bi_next should be NULL
> >
> > I believe it is correct.  Assuming that the list is initially in a
> > sane state, req->biotail->bi_next will be NULL immediately before
> > executing the statement that I added.  Therefore, my fix will set
> > bio->bi_next to NULL, which is what we want because bio becomes the
> > new end of the list.
>
> Yes, but bio->bi_next is a NULL already.

I think assuming this is bad programming form.  You are assuming that
the memory allocator zeros out newly allocated memory.  Though your
assumption may be correct, it's always possible that this behavior will
change some day (perhaps for efficiency reasons), causing your code
to break.  In my opinion, the savings of a few cpu clock cycles that
you gain by omitting the initialization isn't worth compromising
the robustness of your code.

-Dave