Re: [Qemu-devel] [RFC PATCH 1/3] qdev: Add a description field for qdev properties for documentation

[Amit Shah <amit.shah@redhat.com>]

On (Fri) May 28 2010 [14:49:39], Markus Armbruster wrote:
> Amit Shah <amit.shah@redhat.com> writes:
> 
> > Add a 'description' along with each qdev property to document the input
> > each qdev property takes.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> 
> 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