On 09/26/2014 11:14 PM, arei.gong...@huawei.com wrote: > From: Gonglei <arei.gong...@huawei.com> > > Add a new "description" field to DevicePropertyInfo. > The descriptions can serve as documentation in the code, > and they can be used to provide better help. For example: > > $./qemu-system-x86_64 -device virtio-blk-pci,? >
> +++ b/qapi-schema.json
> @@ -1615,11 +1615,12 @@
> #
> # @name: the name of the property
> # @type: the typename of the property
> +# @description: the description of the property
Please add a '(since 2.2)' designation. Will 'description' always be
present (that is, do ALL properties have a description), or is it
optional and only provided when a non-NULL description is available?
[/me reads rest of patch]
Yes, given your code, it looks like you meant for it to be optional.
> #
> # Since: 1.2
> ##
> { 'type': 'DevicePropertyInfo',
> - 'data': { 'name': 'str', 'type': 'str' } }
> + 'data': { 'name': 'str', 'type': 'str', 'description': 'str' } }
Therefore, this needs to be '*description'...
>
> ##
> # @device-list-properties:
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 5ec6606..5ee6bf2 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -213,9 +213,14 @@ int qdev_device_help(QemuOpts *opts)
> }
>
> for (prop = prop_list; prop; prop = prop->next) {
> - error_printf("%s.%s=%s\n", driver,
> + error_printf("%s.%s=%s", driver,
> prop->value->name,
> prop->value->type);
> + if (prop->value->description) {
...and this needs to be 'prop->value->has_description'. We don't
support NULL values of non-optional strings in QMP yet.
> @@ -465,7 +466,9 @@ static DevicePropertyInfo
> *make_device_property_info(ObjectClass *klass,
>
> info = g_malloc0(sizeof(*info));
> info->name = g_strdup(prop->name);
> - info->type = g_strdup(prop->info->legacy_name ?:
> prop->info->name);
> + info->type = g_strdup(prop->info->name);
> + info->description = prop->info->description ?
> + g_strdup(prop->info->description) : NULL;
This needs to be:
info->has_description = !!prop->info->description;
info->description = g_strdup(prop->info->description);
(it's safe to call g_strdup(NULL), avoiding the need for ?: in the
assignment).
> return info;
> }
> klass = object_class_get_parent(klass);
> @@ -475,6 +478,8 @@ static DevicePropertyInfo
> *make_device_property_info(ObjectClass *klass,
> info = g_malloc0(sizeof(*info));
> info->name = g_strdup(name);
> info->type = g_strdup(default_type);
> + info->description = description ? g_strdup(description) : NULL;
Another place that should avoid the ?:, and set has_description.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
