Messages in this thread | | | Date | Thu, 8 May 2003 03:17:17 +0200 (MET DST) | From | Bartlomiej Zolnierkiewicz <> | Subject | Re: [PATCH] fixes for linked list bugs in block I/O code |
| |
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
- 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/
| |