| Message ID | 20190817175346.12518-1-nsoffer@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Use QEMU_IS_ALIGNED instead of reinventing it | expand |
On 8/17/19 1:53 PM, Nir Soffer wrote: > Replace instances of: > > (n & (BDRV_SECTOR_SIZE - 1)) == 0) > > With: > > QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE) > Do you have a magical incantation you used to find these? > Which reveals the intent of the code better, and makes it easier to > locate the code checking alignment. > > QEMU_IS_ALIGNED is implemented using %, which may be less efficient but > it is used only in assert() except one instance, so it should not > matter. > There is virtually no way these aren't optimized by the compiler. > Signed-off-by: Nir Soffer <nsoffer@redhat.com> Therefore: Reviewed-by: John Snow <jsnow@redhat.com> > --- > block/bochs.c | 4 ++-- > block/cloop.c | 4 ++-- > block/dmg.c | 4 ++-- > block/io.c | 8 ++++---- > block/qcow2.c | 4 ++-- > block/vvfat.c | 8 ++++---- > qemu-img.c | 2 +- > 7 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/block/bochs.c b/block/bochs.c > index 962f18592d..32bb83b268 100644 > --- a/block/bochs.c > +++ b/block/bochs.c > @@ -248,8 +248,8 @@ bochs_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, > QEMUIOVector local_qiov; > int ret; > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > qemu_iovec_init(&local_qiov, qiov->niov); > qemu_co_mutex_lock(&s->lock); > diff --git a/block/cloop.c b/block/cloop.c > index 384c9735bb..4de94876d4 100644 > --- a/block/cloop.c > +++ b/block/cloop.c > @@ -253,8 +253,8 @@ cloop_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > int ret, i; > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > qemu_co_mutex_lock(&s->lock); > > diff --git a/block/dmg.c b/block/dmg.c > index 45f6b28f17..4a045f2b3e 100644 > --- a/block/dmg.c > +++ b/block/dmg.c > @@ -697,8 +697,8 @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > int ret, i; > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > qemu_co_mutex_lock(&s->lock); > > diff --git a/block/io.c b/block/io.c > index 56bbf195bb..7508703ecd 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -1080,8 +1080,8 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs, > sector_num = offset >> BDRV_SECTOR_BITS; > nb_sectors = bytes >> BDRV_SECTOR_BITS; > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > assert(bytes <= BDRV_REQUEST_MAX_BYTES); > assert(drv->bdrv_co_readv); > > @@ -1133,8 +1133,8 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs, > sector_num = offset >> BDRV_SECTOR_BITS; > nb_sectors = bytes >> BDRV_SECTOR_BITS; > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > assert(bytes <= BDRV_REQUEST_MAX_BYTES); > > assert(drv->bdrv_co_writev); > diff --git a/block/qcow2.c b/block/qcow2.c > index 59cff1d4cb..41cab70e1d 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -2072,8 +2072,8 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset, > } > if (bs->encrypted) { > assert(s->crypto); > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(cur_bytes, BDRV_SECTOR_SIZE)); > if (qcow2_co_decrypt(bs, cluster_offset, offset, > cluster_data, cur_bytes) < 0) { > ret = -EIO; > diff --git a/block/vvfat.c b/block/vvfat.c > index f6c28805dd..019b8f1341 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -1547,8 +1547,8 @@ vvfat_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > void *buf; > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > buf = g_try_malloc(bytes); > if (bytes && buf == NULL) { > @@ -3082,8 +3082,8 @@ vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes, > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > void *buf; > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > buf = g_try_malloc(bytes); > if (bytes && buf == NULL) { > diff --git a/qemu-img.c b/qemu-img.c > index c920e3564c..ca3af0c549 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -2138,7 +2138,7 @@ static int img_convert(int argc, char **argv) > int64_t sval; > > sval = cvtnum(optarg); > - if (sval < 0 || sval & (BDRV_SECTOR_SIZE - 1) || > + if (sval < 0 || !QEMU_IS_ALIGNED(sval, BDRV_SECTOR_SIZE) || > sval / BDRV_SECTOR_SIZE > MAX_BUF_SECTORS) { > error_report("Invalid buffer size for sparse output specified. " > "Valid sizes are multiples of %llu up to %llu. Select " >
On 17.08.19 19:53, Nir Soffer wrote: > Replace instances of: > > (n & (BDRV_SECTOR_SIZE - 1)) == 0) > > With: > > QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE) > > Which reveals the intent of the code better, and makes it easier to > locate the code checking alignment. > > QEMU_IS_ALIGNED is implemented using %, which may be less efficient but > it is used only in assert() except one instance, so it should not > matter. > > Signed-off-by: Nir Soffer <nsoffer@redhat.com> > --- > block/bochs.c | 4 ++-- > block/cloop.c | 4 ++-- > block/dmg.c | 4 ++-- > block/io.c | 8 ++++---- > block/qcow2.c | 4 ++-- > block/vvfat.c | 8 ++++---- > qemu-img.c | 2 +- > 7 files changed, 17 insertions(+), 17 deletions(-) Because John was speaking about a magical incantation, I found BDRV_SECTOR_MASK. There are two places in block/qcow2-cluster.c that use that to check alignment, I think those should be amended as well. Max
On Tue, Aug 20, 2019 at 10:30 PM John Snow <jsnow@redhat.com> wrote: > > > On 8/17/19 1:53 PM, Nir Soffer wrote: > > Replace instances of: > > > > (n & (BDRV_SECTOR_SIZE - 1)) == 0) > > > > With: > > > > QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE) > > > > Do you have a magical incantation you used to find these? > I found the first by accident, then I used: git grep 'BDRV_SECTOR_SIZE - 1' > Which reveals the intent of the code better, and makes it easier to > > locate the code checking alignment. > > > > QEMU_IS_ALIGNED is implemented using %, which may be less efficient but > > it is used only in assert() except one instance, so it should not > > matter. > > > > There is virtually no way these aren't optimized by the compiler. > Makes sense, but I did not check. > > > Signed-off-by: Nir Soffer <nsoffer@redhat.com> > > Therefore: > > Reviewed-by: John Snow <jsnow@redhat.com> > > > --- > > block/bochs.c | 4 ++-- > > block/cloop.c | 4 ++-- > > block/dmg.c | 4 ++-- > > block/io.c | 8 ++++---- > > block/qcow2.c | 4 ++-- > > block/vvfat.c | 8 ++++---- > > qemu-img.c | 2 +- > > 7 files changed, 17 insertions(+), 17 deletions(-) > > > > diff --git a/block/bochs.c b/block/bochs.c > > index 962f18592d..32bb83b268 100644 > > --- a/block/bochs.c > > +++ b/block/bochs.c > > @@ -248,8 +248,8 @@ bochs_co_preadv(BlockDriverState *bs, uint64_t > offset, uint64_t bytes, > > QEMUIOVector local_qiov; > > int ret; > > > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > > > qemu_iovec_init(&local_qiov, qiov->niov); > > qemu_co_mutex_lock(&s->lock); > > diff --git a/block/cloop.c b/block/cloop.c > > index 384c9735bb..4de94876d4 100644 > > --- a/block/cloop.c > > +++ b/block/cloop.c > > @@ -253,8 +253,8 @@ cloop_co_preadv(BlockDriverState *bs, uint64_t > offset, uint64_t bytes, > > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > > int ret, i; > > > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > > > qemu_co_mutex_lock(&s->lock); > > > > diff --git a/block/dmg.c b/block/dmg.c > > index 45f6b28f17..4a045f2b3e 100644 > > --- a/block/dmg.c > > +++ b/block/dmg.c > > @@ -697,8 +697,8 @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, > uint64_t bytes, > > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > > int ret, i; > > > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > > > qemu_co_mutex_lock(&s->lock); > > > > diff --git a/block/io.c b/block/io.c > > index 56bbf195bb..7508703ecd 100644 > > --- a/block/io.c > > +++ b/block/io.c > > @@ -1080,8 +1080,8 @@ static int coroutine_fn > bdrv_driver_preadv(BlockDriverState *bs, > > sector_num = offset >> BDRV_SECTOR_BITS; > > nb_sectors = bytes >> BDRV_SECTOR_BITS; > > > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > assert(bytes <= BDRV_REQUEST_MAX_BYTES); > > assert(drv->bdrv_co_readv); > > > > @@ -1133,8 +1133,8 @@ static int coroutine_fn > bdrv_driver_pwritev(BlockDriverState *bs, > > sector_num = offset >> BDRV_SECTOR_BITS; > > nb_sectors = bytes >> BDRV_SECTOR_BITS; > > > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > assert(bytes <= BDRV_REQUEST_MAX_BYTES); > > > > assert(drv->bdrv_co_writev); > > diff --git a/block/qcow2.c b/block/qcow2.c > > index 59cff1d4cb..41cab70e1d 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > @@ -2072,8 +2072,8 @@ static coroutine_fn int > qcow2_co_preadv(BlockDriverState *bs, uint64_t offset, > > } > > if (bs->encrypted) { > > assert(s->crypto); > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(cur_bytes, BDRV_SECTOR_SIZE)); > > if (qcow2_co_decrypt(bs, cluster_offset, offset, > > cluster_data, cur_bytes) < 0) { > > ret = -EIO; > > diff --git a/block/vvfat.c b/block/vvfat.c > > index f6c28805dd..019b8f1341 100644 > > --- a/block/vvfat.c > > +++ b/block/vvfat.c > > @@ -1547,8 +1547,8 @@ vvfat_co_preadv(BlockDriverState *bs, uint64_t > offset, uint64_t bytes, > > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > > void *buf; > > > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > > > buf = g_try_malloc(bytes); > > if (bytes && buf == NULL) { > > @@ -3082,8 +3082,8 @@ vvfat_co_pwritev(BlockDriverState *bs, uint64_t > offset, uint64_t bytes, > > int nb_sectors = bytes >> BDRV_SECTOR_BITS; > > void *buf; > > > > - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); > > - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); > > + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); > > + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); > > > > buf = g_try_malloc(bytes); > > if (bytes && buf == NULL) { > > diff --git a/qemu-img.c b/qemu-img.c > > index c920e3564c..ca3af0c549 100644 > > --- a/qemu-img.c > > +++ b/qemu-img.c > > @@ -2138,7 +2138,7 @@ static int img_convert(int argc, char **argv) > > int64_t sval; > > > > sval = cvtnum(optarg); > > - if (sval < 0 || sval & (BDRV_SECTOR_SIZE - 1) || > > + if (sval < 0 || !QEMU_IS_ALIGNED(sval, BDRV_SECTOR_SIZE) || > > sval / BDRV_SECTOR_SIZE > MAX_BUF_SECTORS) { > > error_report("Invalid buffer size for sparse output > specified. " > > "Valid sizes are multiples of %llu up to %llu. > Select " > > > > >
On Tue, Aug 20, 2019 at 10:51 PM Max Reitz <mreitz@redhat.com> wrote: > On 17.08.19 19:53, Nir Soffer wrote: > > Replace instances of: > > > > (n & (BDRV_SECTOR_SIZE - 1)) == 0) > > > > With: > > > > QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE) > > > > Which reveals the intent of the code better, and makes it easier to > > locate the code checking alignment. > > > > QEMU_IS_ALIGNED is implemented using %, which may be less efficient but > > it is used only in assert() except one instance, so it should not > > matter. > > > > Signed-off-by: Nir Soffer <nsoffer@redhat.com> > > --- > > block/bochs.c | 4 ++-- > > block/cloop.c | 4 ++-- > > block/dmg.c | 4 ++-- > > block/io.c | 8 ++++---- > > block/qcow2.c | 4 ++-- > > block/vvfat.c | 8 ++++---- > > qemu-img.c | 2 +- > > 7 files changed, 17 insertions(+), 17 deletions(-) > > Because John was speaking about a magical incantation, I found > BDRV_SECTOR_MASK. There are two places in block/qcow2-cluster.c that > use that to check alignment, I think those should be amended as well. > $ git grep BDRV_SECTOR_MASK block/qcow2-cluster.c: assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0); block/qcow2-cluster.c: assert((bytes & ~BDRV_SECTOR_MASK) == 0); Right, should use QEMU_IS_ALIGNED include/block/block.h:#define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1) include/block/block.h:#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK migration/block.c: flags = addr & ~BDRV_SECTOR_MASK; This could use (BDRV_SECTOR_SIZE - 1). In Linux SECTOR_MASK is defined as: drivers/block/null_blk_main.c:#define SECTOR_MASK (PAGE_SECTORS - 1) It seems that qemu use use the same. I will try to handle these in v2. Nir
diff --git a/block/bochs.c b/block/bochs.c index 962f18592d..32bb83b268 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -248,8 +248,8 @@ bochs_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, QEMUIOVector local_qiov; int ret; - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); qemu_iovec_init(&local_qiov, qiov->niov); qemu_co_mutex_lock(&s->lock); diff --git a/block/cloop.c b/block/cloop.c index 384c9735bb..4de94876d4 100644 --- a/block/cloop.c +++ b/block/cloop.c @@ -253,8 +253,8 @@ cloop_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, int nb_sectors = bytes >> BDRV_SECTOR_BITS; int ret, i; - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); qemu_co_mutex_lock(&s->lock); diff --git a/block/dmg.c b/block/dmg.c index 45f6b28f17..4a045f2b3e 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -697,8 +697,8 @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, int nb_sectors = bytes >> BDRV_SECTOR_BITS; int ret, i; - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); qemu_co_mutex_lock(&s->lock); diff --git a/block/io.c b/block/io.c index 56bbf195bb..7508703ecd 100644 --- a/block/io.c +++ b/block/io.c @@ -1080,8 +1080,8 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs, sector_num = offset >> BDRV_SECTOR_BITS; nb_sectors = bytes >> BDRV_SECTOR_BITS; - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); assert(bytes <= BDRV_REQUEST_MAX_BYTES); assert(drv->bdrv_co_readv); @@ -1133,8 +1133,8 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs, sector_num = offset >> BDRV_SECTOR_BITS; nb_sectors = bytes >> BDRV_SECTOR_BITS; - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); assert(bytes <= BDRV_REQUEST_MAX_BYTES); assert(drv->bdrv_co_writev); diff --git a/block/qcow2.c b/block/qcow2.c index 59cff1d4cb..41cab70e1d 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2072,8 +2072,8 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset, } if (bs->encrypted) { assert(s->crypto); - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(cur_bytes, BDRV_SECTOR_SIZE)); if (qcow2_co_decrypt(bs, cluster_offset, offset, cluster_data, cur_bytes) < 0) { ret = -EIO; diff --git a/block/vvfat.c b/block/vvfat.c index f6c28805dd..019b8f1341 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1547,8 +1547,8 @@ vvfat_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, int nb_sectors = bytes >> BDRV_SECTOR_BITS; void *buf; - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); buf = g_try_malloc(bytes); if (bytes && buf == NULL) { @@ -3082,8 +3082,8 @@ vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes, int nb_sectors = bytes >> BDRV_SECTOR_BITS; void *buf; - assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0); - assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0); + assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); buf = g_try_malloc(bytes); if (bytes && buf == NULL) { diff --git a/qemu-img.c b/qemu-img.c index c920e3564c..ca3af0c549 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2138,7 +2138,7 @@ static int img_convert(int argc, char **argv) int64_t sval; sval = cvtnum(optarg); - if (sval < 0 || sval & (BDRV_SECTOR_SIZE - 1) || + if (sval < 0 || !QEMU_IS_ALIGNED(sval, BDRV_SECTOR_SIZE) || sval / BDRV_SECTOR_SIZE > MAX_BUF_SECTORS) { error_report("Invalid buffer size for sparse output specified. " "Valid sizes are multiples of %llu up to %llu. Select "
Replace instances of: (n & (BDRV_SECTOR_SIZE - 1)) == 0) With: QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE) Which reveals the intent of the code better, and makes it easier to locate the code checking alignment. QEMU_IS_ALIGNED is implemented using %, which may be less efficient but it is used only in assert() except one instance, so it should not matter. Signed-off-by: Nir Soffer <nsoffer@redhat.com> --- block/bochs.c | 4 ++-- block/cloop.c | 4 ++-- block/dmg.c | 4 ++-- block/io.c | 8 ++++---- block/qcow2.c | 4 ++-- block/vvfat.c | 8 ++++---- qemu-img.c | 2 +- 7 files changed, 17 insertions(+), 17 deletions(-)