Re: [Qemu-devel] [PATCH V3 01/11] qemu-img: remove unused parameter in collect_image_info()

[Wenchao Xia]

于 2013-1-15 15:27, Wenchao Xia 写道:
> 于 2013-1-15 1:08, Luiz Capitulino 写道:
> > On Mon, 14 Jan 2013 15:09:37 +0800
> > Wenchao Xia <address@hidden> wrote:
> >
> > >    Parameter *fmt was not used, so remove it.
> > >
> > > Reviewed-by: Eric Blake <address@hidden>
> > > Signed-off-by: Wenchao Xia <address@hidden>
> > > ---
> > >   qemu-img.c |    5 ++---
> > >   1 files changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/qemu-img.c b/qemu-img.c
> > > index 85d3740..9dab48f 100644
> > > --- a/qemu-img.c
> > > +++ b/qemu-img.c
> > > @@ -1186,8 +1186,7 @@ static void dump_json_image_info(ImageInfo *info)
> > >
> > >   static void collect_image_info(BlockDriverState *bs,
> > >                      ImageInfo *info,
> > > -                   const char *filename,
> > > -                   const char *fmt)
> > > +                   const char *filename)
> >
> > collect_image_info_list() doc reads:
> >
> >   @fmt: topmost image format (may be NULL to autodetect)
> >
> > However, right now only fmt=NULL is supported, as collect_image_info()
> > ignores fmt altogether.
> >
> > So, if this patch is correct we better update the comment. Otherwise,
> > we should improve collect_image_info() to actually obey fmt != NULL.
>    @fmt was ignored in the function and I can't see a reason to have
> it while *bs contains the info, will change the comments.
  Hi, *fmt was used only in collect_image_info_list() when it tries to
open the image, and it is not useful any more in collect_image_info,
so nothing need change in comments.

> > >   {
> > >       uint64_t total_sectors;
> > >       char backing_filename[1024];
> > > @@ -1361,7 +1360,7 @@ static ImageInfoList
> > > *collect_image_info_list(const char *filename,
> > >           }
> > >
> > >           info = g_new0(ImageInfo, 1);
> > > -        collect_image_info(bs, info, filename, fmt);
> > > +        collect_image_info(bs, info, filename);
> > >           collect_snapshots(bs, info);
> > >
> > >           elem = g_new0(ImageInfoList, 1);


--
Best Regards

Wenchao Xia