|
From: | Wenchao Xia |
Subject: | Re: [Qemu-devel] [PATCH V3 01/11] qemu-img: remove unused parameter in collect_image_info() |
Date: | Wed, 16 Jan 2013 11:10:14 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130107 Thunderbird/17.0.2 |
于 2013-1-15 19:11, Luiz Capitulino 写道:
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.
I think the "fmt may be NULL or non-NULL" indeed have different behavior for that later following is called: bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING, false); but it is not related to collect_image_info(), it is more like a slip in coding having add *fmt in above funtion. :) -- Best Regards Wenchao Xia
[Prev in Thread] | Current Thread | [Next in Thread] |