Re: [Qemu-devel] [PATCH v2 09/10] qemu-img: Make img_convert() get image size just once per image

[Max Reitz]

On 30.05.2014 08:56, Markus Armbruster wrote:
> Max Reitz <address@hidden> writes:
>
> > On 28.05.2014 16:25, Markus Armbruster wrote:
> > > Chiefly so I don't have to do the error checking in quadruplicate in
> > > the next commit.  Moreover, replacing the frequently updated
> > > bs_sectors by an array assigned just once makes the code easier to
> > > understand.
> > >
> > > Signed-off-by: Markus Armbruster <address@hidden>
> > > ---
> > >    qemu-img.c | 32 ++++++++++++++++----------------
> > >    1 file changed, 16 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/qemu-img.c b/qemu-img.c
> > > index 8d996ba..229c0c6 100644
> > > --- a/qemu-img.c
> > > +++ b/qemu-img.c
> > > @@ -1186,7 +1186,7 @@ static int img_convert(int argc, char **argv)
> > >        BlockDriver *drv, *proto_drv;
> > >        BlockDriverState **bs = NULL, *out_bs = NULL;
> > >        int64_t total_sectors, nb_sectors, sector_num, bs_offset;
> > > -    uint64_t bs_sectors;
> > > +    uint64_t *bs_sectors = NULL;
> > >        uint8_t * buf = NULL;
> > >        size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE;
> > >        const uint8_t *buf1;
> > > @@ -1326,7 +1326,8 @@ static int img_convert(int argc, char **argv)
> > >          qemu_progress_print(0, 100);
> > >    -    bs = g_malloc0(bs_n * sizeof(BlockDriverState *));
> > > +    bs = g_new(BlockDriverState *, bs_n);
> > I think this should rather be g_new0(), so the clean-up code after
> > "out:" doesn't get confused about which BDS are valid and which are
> > not.
> You're right.
>
> I'm afraid manually converting to g_new() & friends when I touch the
> line anyway is too error prone for me.  I'll dust off the block parts of
> my "Use g_new() & friends where that makes obvious sense" series.
>
> > Other than that, this patch looks good to me.
> Thanks!  May I add your R-by if all I change is fixing the g_new() to
> g_new0()?

Yes, feel free to. :-)

Max