On Tue, May 05, 2015 at 10:51:14AM +0800, Fam Zheng wrote: This function is complex. I had to draw a diagram to remember the relationships between the variables. It would be nice to split it if that can be done in a way that makes the code nicer.
> @@ -1236,13 +1238,39 @@ static int coroutine_fn > bdrv_co_do_pwritev(BlockDriverState *bs, > } > BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD); > > - qemu_iovec_init(&local_qiov, qiov->niov + 2); > - qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1)); > - qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size); > - use_local_qiov = true; > + if (qiov) { > + qemu_iovec_init(&local_qiov, qiov ? qiov->niov + 2 : 1); Should be: qemu_iovec_init(&local_qiov, qiov->niov + 2); since we know qiov != NULL here. > @@ -1270,21 +1298,39 @@ static int coroutine_fn > bdrv_co_do_pwritev(BlockDriverState *bs, > } > BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL); > > - if (!use_local_qiov) { > - qemu_iovec_init(&local_qiov, qiov->niov + 1); > - qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size); > - use_local_qiov = true; > + if (qiov) { > + if (!use_local_qiov) { > + qemu_iovec_init(&local_qiov, qiov->niov + 1); > + qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size); > + use_local_qiov = true; > + } > + > + tail_bytes = (offset + bytes) & (align - 1); > + qemu_iovec_add(&local_qiov, tail_buf + tail_bytes, > + align - tail_bytes); > + > + bytes = ROUND_UP(bytes, align); > + } else { > + assert((offset & (align - 1)) == 0); > + assert(bytes < align); > + > + memset(tail_buf, 0, bytes & (align - 1)); Since assert(bytes < align) is above, the following is clearer: memset(tail_buf, 0, bytes)
pgphlpwuFsq7I.pgp
Description: PGP signature