[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V3 01/11] qemu-img: remove unused parameter in c
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH V3 01/11] qemu-img: remove unused parameter in collect_image_info() |
Date: |
Tue, 15 Jan 2013 09:11:09 -0200 |
On Tue, 15 Jan 2013 15:58:34 +0800
Wenchao Xia <address@hidden> wrote:
> 于 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.
This really doesn't answer my comment above. The code comment implies that
fmt may be NULL or non-NULL and they have different behavior.
If you choose to touch fmt (as this patch does) then please, do the
right thing. Otherwise it's better to let it untouched.
[Qemu-devel] [PATCH V3 02/11] block: add bdrv_get_filename() function, Wenchao Xia, 2013/01/14
[Qemu-devel] [PATCH V3 05/11] block: rename bdrv_query_info to bdrv_query_block_info, Wenchao Xia, 2013/01/14
[Qemu-devel] [PATCH V3 03/11] block: add snapshot and image info query function, Wenchao Xia, 2013/01/14
[Qemu-devel] [PATCH V3 04/11] qemu-img: switch image retrieving function, Wenchao Xia, 2013/01/14