[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 01/18] block: Make blk_{pread, pwrite}() return 0 on success
From: |
Alberto Faria |
Subject: |
Re: [PATCH 01/18] block: Make blk_{pread, pwrite}() return 0 on success |
Date: |
Mon, 4 Jul 2022 17:20:25 +0100 |
On Mon, Jul 4, 2022 at 2:52 PM Hanna Reitz <hreitz@redhat.com> wrote:
> There are a couple of places where you decided to replace “*len”
> variables that used to store the return value by a plain “ret”. That
> seems good to me, given how these functions no longer return length
> values, but you haven’t done so consistently. Below, I have noted
> places where this wasn’t done; I wonder why, but my R-b stands
> regardless of whether you change them too or not.
Thanks, this was just an oversight on my part. I'll fix it.
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 4cf4d2423d..9d98ef63ac 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -5120,30 +5120,27 @@ static int img_dd(int argc, char **argv)
> > in.buf = g_new(uint8_t, in.bsz);
> >
> > for (out_pos = 0; in_pos < size; block_count++) {
> > - int in_ret, out_ret;
> > + int bytes, in_ret, out_ret;
> >
> > - if (in_pos + in.bsz > size) {
> > - in_ret = blk_pread(blk1, in_pos, in.buf, size - in_pos);
> > - } else {
> > - in_ret = blk_pread(blk1, in_pos, in.buf, in.bsz);
> > - }
> > + bytes = (in_pos + in.bsz > size) ? size - in_pos : in.bsz;
> > +
> > + in_ret = blk_pread(blk1, in_pos, in.buf, bytes);
> > if (in_ret < 0) {
> > error_report("error while reading from input image file: %s",
> > strerror(-in_ret));
> > ret = -1;
> > goto out;
> > }
> > - in_pos += in_ret;
> > -
> > - out_ret = blk_pwrite(blk2, out_pos, in.buf, in_ret, 0);
> > + in_pos += bytes;
> >
> > + out_ret = blk_pwrite(blk2, out_pos, in.buf, bytes, 0);
> > if (out_ret < 0) {
> > error_report("error while writing to output image file: %s",
> > strerror(-out_ret));
> > ret = -1;
> > goto out;
> > }
> > - out_pos += out_ret;
> > + out_pos += bytes;
> > }
> >
> > out:
>
> We could use this opportunity to drop in_ret and out_ret altogether (but
> we can also decide not to).
Dropping them sounds good to me.
Alberto