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

[Bartlomiej Zolnierkiewicz]

On Wed, 7 May 2003, Dave Peterson wrote:

> 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

No, it is not memory allocator but block layer.
Look at bio_init(), there is bio->bi_next = NULL explicitly.

> 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.

Agreed, but not the case here (speaking 2.5).
Better add check for bio->bi_next != NULL to catch improper usage.

--
Bartlomiej

> -Dave