[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 1/3] qdev: Add a description field for qdev
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/3] qdev: Add a description field for qdev properties for documentation |
Date: |
Fri, 28 May 2010 18:27:29 +0530 |
User-agent: |
Mutt/1.5.20 (2009-12-10) |
On (Fri) May 28 2010 [14:49:39], Markus Armbruster wrote:
> Amit Shah <address@hidden> writes:
>
> > Add a 'description' along with each qdev property to document the input
> > each qdev property takes.
> >
> > Signed-off-by: Amit Shah <address@hidden>
>
> 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