From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53737) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QA7AL-00027u-Dq for qemu-devel@nongnu.org; Wed, 13 Apr 2011 16:59:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QA7AK-0000Q0-F4 for qemu-devel@nongnu.org; Wed, 13 Apr 2011 16:59:57 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:58619) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QA7AK-0000Pw-7z for qemu-devel@nongnu.org; Wed, 13 Apr 2011 16:59:56 -0400 Received: by yxk8 with SMTP id 8so535732yxk.4 for ; Wed, 13 Apr 2011 13:59:55 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20110411083304.GB22987@stefanha-thinkpad.localdomain> References: <20110411083304.GB22987@stefanha-thinkpad.localdomain> Date: Thu, 14 Apr 2011 04:59:55 +0800 Message-ID: From: Lyu Mitnick Content-Type: multipart/alternative; boundary=001485f9a4e03f115d04a0d317c7 Subject: Re: [Qemu-devel] Question about total_sectors in block/vpc.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Christoph Hellwig --001485f9a4e03f115d04a0d317c7 Content-Type: text/plain; charset=ISO-8859-1 Hello Stefan, I have a question about get_option_parameter(). I am wondering whether get_option_parameter is suitable to use instead of doing the search by myself in the case like following: /* Read out options */ while (options && options->name) { if (!strcmp(options->name, BLOCK_OPT_SIZE)) { // do something } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { // do something } options++; } Thanks 2011/4/11 Stefan Hajnoczi > On Sun, Apr 10, 2011 at 05:02:20PM +0800, Lyu Mitnick wrote: > > diff --git a/block.c b/block.c > > index f731c7a..a80ec49 100644 > > --- a/block.c > > +++ b/block.c > > @@ -239,6 +239,16 @@ int bdrv_create(BlockDriver *drv, const char* > filename, > > if (!drv->bdrv_create) > > return -ENOTSUP; > > > > + while (options && options->name) { > > + if (!strcmp(options->name, "size")) { > > + if (options->value.n % 512 == 0) > > + break; > > + else > > + return -EINVAL; > > + } > > + options++; > > + } > > Please use BDRV_SECTOR_SIZE instead of hardcoding 512. > > get_option_parameter() does the search for you, please use it instead of > duplicating the loop. > > Please see the CODING_STYLE and HACKING files, new code should follow it: > * Indentation is 4 spaces > * Always use {} even for if/else with single-statement bodies > > Stefan > Mitnick --001485f9a4e03f115d04a0d317c7 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Hello Stefan,

I have a question about=A0get_option_param= eter(). I am wondering whether=A0
get_option_parameter =A0is=A0su= itable to use=A0instead of=A0doing the search by myself=A0
in the= case like following:

/* Read out options */
=A0=A0 =A0while (= options && options->name) {
=A0=A0 =A0 =A0 =A0if (!str= cmp(options->name, BLOCK_OPT_SIZE)) {
=A0=A0 =A0 =A0 =A0 =A0 = =A0// do something
=A0=A0 =A0 =A0 =A0} else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SI= ZE)) {
=A0=A0 =A0 =A0 =A0 =A0 // do something
=A0=A0 = =A0 =A0 =A0}
=A0=A0 =A0 =A0 =A0options++;
=A0=A0 =A0}

Thanks

2011/4/11 Stefan Hajnoczi = <stefanha@gmail.com>=
On Sun, Apr 10, 2011 at 05:02:20PM +0800, Lyu Mitnick wro= te:
> diff --git a/block.c b/block.c
> index f731c7a..a80ec49 100644
> --- a/block.c
> +++ b/block.c
> @@ -239,6 +239,16 @@ int bdrv_create(BlockDriver *drv, const char* fil= ename,
> =A0 =A0 if (!drv->bdrv_create)
> =A0 =A0 =A0 =A0 return -ENOTSUP;
>
> + =A0 =A0 =A0 while (options && options->name) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!strcmp(options->name, "size= ")) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (options->value.n = % 512 =3D=3D 0)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -= EINVAL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 options++;
> + =A0 =A0 =A0 }

Please use BDRV_SECTOR_SIZE instead of hardcoding 512.

get_option_parameter() does the search for you, please use it instead of duplicating the loop.

Please see the CODING_STYLE and HACKING files, new code should follow it: =A0* Indentation is 4 spaces
=A0* Always use {} even for if/else with single-statement bodies

Stefan

Mitnick
--001485f9a4e03f115d04a0d317c7--