[Wasn't delivered correctly by eggs.gnu.org, resending] The subsystem tag should be just "qdev:". Suggest "qdev: Clean up around properties".
Cao jin <caoj.f...@cn.fujitsu.com> writes: > include: > 1. remove unnecessary declaration of static function > 2. fix inconsistency between comment and function name, and typo OOM->QOM > 2. update comment of function > > Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> > --- > A question about legacy property: I don`t see legacy property is really > used in the code, so, why still add legacy property to object? > > since it has no maintainer, so the cc list is by intuition... > > hw/core/qdev.c | 14 ++++++-------- > include/hw/qdev-properties.h | 4 ++-- > 2 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index db41aa1..743b71b 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -58,9 +58,6 @@ const char *qdev_fw_name(DeviceState *dev) > return object_get_typename(OBJECT(dev)); > } > > -static void qdev_property_add_legacy(DeviceState *dev, Property *prop, > - Error **errp); > - > static void bus_remove_child(BusState *bus, DeviceState *child) > { > BusChild *kid; > @@ -908,12 +905,12 @@ static void qdev_get_legacy_property(Object *obj, > Visitor *v, > } > > /** > - * @qdev_add_legacy_property - adds a legacy property > + * @qdev_property_add_legacy - adds a legacy property Style#1: function_name - headline We generally use the imperative mood: "add a legacy property". > * > * Do not use this is new code! Properties added through this interface will > * be given names and types in the "legacy" namespace. > * > - * Legacy properties are string versions of other OOM properties. The format > + * Legacy properties are string versions of other QOM properties. The format > * of the string depends on the property type. > */ > static void qdev_property_add_legacy(DeviceState *dev, Property *prop, > @@ -937,10 +934,11 @@ static void qdev_property_add_legacy(DeviceState *dev, > Property *prop, > } > > /** > - * @qdev_property_add_static - add a @Property to a device. > + * @qdev_property_add_static > + * add a QOM property to @dev via its qdev Property @prop Style#2: function_name headline > * > - * Static properties access data in a struct. The actual type of the > - * property and the field depends on the property type. > + * Static properties access data in a struct. The actual type of > ObjectProperty > + * and the struct field depends on the @qtype type. > */ Not sure this part is an improvement. What's wrong with the current text? > void qdev_property_add_static(DeviceState *dev, Property *prop, > Error **errp) > diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h > index 0586cac..8c4481b 100644 > --- a/include/hw/qdev-properties.h > +++ b/include/hw/qdev-properties.h > @@ -197,8 +197,8 @@ void error_set_from_qdev_prop_error(Error **errp, int > ret, DeviceState *dev, > Property *prop, const char *value); > > /** > - * @qdev_property_add_static - add a @Property to a device referencing a > - * field in a struct. > + * @qdev_property_add_static - add a ObjectProperty to @dev via its qdev > + * property @prop, referencing a field in the struct. > */ Style#3: function_name - sentence spanning multiple lines. Pick one style and stick to it. The rest of qdev-properties.h appears to use GTK-Doc style (which I personally dislike, but that doesn't matter here). See https://developer.gnome.org/gtk-doc-manual/stable/documenting_symbols.html If both declaration and definition have a comment, as they do here, they should match exactly. > void qdev_property_add_static(DeviceState *dev, Property *prop, Error > **errp);