From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34841) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvXDk-000541-Mv for qemu-devel@nongnu.org; Wed, 16 Jan 2013 12:56:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TvXDj-0007Xo-Hf for qemu-devel@nongnu.org; Wed, 16 Jan 2013 12:56:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41811) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvXDj-0007Xd-AT for qemu-devel@nongnu.org; Wed, 16 Jan 2013 12:56:15 -0500 Date: Wed, 16 Jan 2013 15:56:18 -0200 From: Luiz Capitulino Message-ID: <20130116155618.0b13c282@doriath.home> In-Reply-To: <50F61A16.1050200@linux.vnet.ibm.com> References: <1358147387-8221-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1358147387-8221-2-git-send-email-xiawenc@linux.vnet.ibm.com> <20130114150811.63fe1455@doriath.home> <50F504CF.4020607@linux.vnet.ibm.com> <50F50C2A.3010400@linux.vnet.ibm.com> <20130115091109.357a944c@doriath.home> <50F61A16.1050200@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V3 01/11] qemu-img: remove unused parameter in collect_image_info() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: aliguori@us.ibm.com, phrdina@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com On Wed, 16 Jan 2013 11:10:14 +0800 Wenchao Xia wrote: > =E4=BA=8E 2013-1-15 19:11, Luiz Capitulino =E5=86=99=E9=81=93: > > On Tue, 15 Jan 2013 15:58:34 +0800 > > Wenchao Xia wrote: > > > >> =E4=BA=8E 2013-1-15 15:27, Wenchao Xia =E5=86=99=E9=81=93: > >>> =E4=BA=8E 2013-1-15 1:08, Luiz Capitulino =E5=86=99=E9=81=93: > >>>> On Mon, 14 Jan 2013 15:09:37 +0800 > >>>> Wenchao Xia wrote: > >>>> > >>>>> Parameter *fmt was not used, so remove it. > >>>>> > >>>>> Reviewed-by: Eric Blake > >>>>> Signed-off-by: Wenchao Xia > >>>>> --- > >>>>> 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 *i= nfo) > >>>>> > >>>>> 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=3DNULL is supported, as collect_image_in= fo() > >>>> 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 !=3D NUL= L. > >>>> > >>> @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 t= hat > > 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 =3D 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. :) Oh, you seem to be right. Sorry for the noise.