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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed May 07 2003 - 22:00:34 EST