On Tue, 15 Jan 2013 15:58:34 +0800 Wenchao Xia <xiaw...@linux.vnet.ibm.com> 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 <xiaw...@linux.vnet.ibm.com> wrote: > >> > >>> Parameter *fmt was not used, so remove it. > >>> > >>> Reviewed-by: Eric Blake <ebl...@redhat.com> > >>> Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com> > >>> --- > >>> 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.
