From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=37794 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OHz6L-0008TZ-1z for qemu-devel@nongnu.org; Fri, 28 May 2010 08:55:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OHz6J-0002IK-QX for qemu-devel@nongnu.org; Fri, 28 May 2010 08:55:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56414) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OHz6J-0002ID-Hz for qemu-devel@nongnu.org; Fri, 28 May 2010 08:55:47 -0400 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4SCtiId010352 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 28 May 2010 08:55:46 -0400 Date: Fri, 28 May 2010 18:27:29 +0530 From: Amit Shah Subject: Re: [Qemu-devel] [RFC PATCH 1/3] qdev: Add a description field for qdev properties for documentation Message-ID: <20100528125729.GA9122@amit-laptop.redhat.com> References: <24764499566cd2edeb02b44be2d53150e076d5b5.1275040675.git.amit.shah@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Gerd Hoffmann , qemu list , Juan Quintela On (Fri) May 28 2010 [14:49:39], Markus Armbruster wrote: > Amit Shah writes: > > > Add a 'description' along with each qdev property to document the input > > each qdev property takes. > > > > Signed-off-by: Amit Shah > > I always wanted this, and never got around to code it up. Thanks! Cool, thanks! > [...] > > diff --git a/block_int.h b/block_int.h > > index 1a7240c..767cd29 100644 > > --- a/block_int.h > > +++ b/block_int.h > > @@ -231,12 +231,12 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) > > } > > > > #define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ > > - DEFINE_PROP_DRIVE("drive", _state, _conf.dinfo), \ > > + DEFINE_PROP_DRIVE("drive", _state, _conf.dinfo, ""), \ > > DEFINE_PROP_UINT16("logical_block_size", _state, \ > > - _conf.logical_block_size, 512), \ > > + _conf.logical_block_size, 512, ""), \ > > DEFINE_PROP_UINT16("physical_block_size", _state, \ > > - _conf.physical_block_size, 512), \ > > - DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 512), \ > > - DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 512) > > + _conf.physical_block_size, 512, ""), \ > > + DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 512, ""), \ > > + DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 512, "") > > NULL feels more natural than "" for missing description. "" is shorter than NULL, which surprisingly saves quite a few line splits to keep them below 80 chars wide. Also, the hope is to quickly fill them all out with descriptions in follow-up patches, so this shouldn't be a big issue. > [...] > > diff --git a/hw/qdev.c b/hw/qdev.c > > index af17486..2cd205b 100644 > > --- a/hw/qdev.c > > +++ b/hw/qdev.c > > @@ -187,7 +187,8 @@ int qdev_device_help(QemuOpts *opts) > > if (!prop->info->parse) { > > continue; /* no way to set it, don't show */ > > } > > - error_printf("%s.%s=%s\n", info->name, prop->name, prop->info->name); > > + error_printf("%s.%s=%s, %s\n", info->name, prop->name, > > + prop->info->name, prop->desc ?: ""); > > The ?: operator a gcc extension. Do we care? Again, this is shorter and we're already using lots of gcc extensions.. Amit