diff mbox

[07/16] qdev: allow reusing get/set for legacy property

Message ID 1328201142-26145-8-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Feb. 2, 2012, 4:45 p.m. UTC 
In some cases, a legacy property does need a special print method
but not a special parse method.  In this case, we can reuse the get/set
from the static (non-legacy) property.

If neither parse nor print is needed, though, do not register the
legacy property at all.  The previous patch ensures that the right
fallback will be used.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/qdev-monitor.c    |    5 ++---
 hw/qdev-properties.c |    6 +++---
 hw/qdev.c            |   11 +++++++----
 3 files changed, 12 insertions(+), 10 deletions(-)

Comments

Andreas Färber Feb. 2, 2012, 10:38 p.m. UTC | #1 
Am 02.02.2012 17:45, schrieb Paolo Bonzini:
> In some cases, a legacy property does need a special print method
> but not a special parse method.  In this case, we can reuse the get/set
> from the static (non-legacy) property.
> 
> If neither parse nor print is needed, though, do not register the
> legacy property at all.  The previous patch ensures that the right
> fallback will be used.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/qdev-monitor.c    |    5 ++---
>  hw/qdev-properties.c |    6 +++---
>  hw/qdev.c            |   11 +++++++----
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 64505b4..e21bd50 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -489,8 +489,8 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
>  {
>      if (!props)
>          return;
> -    while (props->name) {
> -        Error *err;
> +    for (; props->name; props++) {
> +        Error *err = NULL;

Do either of these fix a bug? Should be mentioned or avoided.

>          char *value;
>          char *legacy_name = g_strdup_printf("legacy-%s", props->name);
>          if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
> @@ -507,7 +507,6 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
>          qdev_printf("%s-prop: %s = %s\n", prefix, props->name,
>                      value && *value ? value : "<null>");
>          g_free(value);
> -        props++;
>      }
>  }
>  
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 7c41140..16f9b22 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -1025,13 +1025,13 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
>  int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
>  {
>      char *legacy_name;
> -    Error *err;
> +    Error *err = NULL;
>  
>      legacy_name = g_strdup_printf("legacy-%s", name);
>      if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
> -        object_property_set_str(OBJECT(dev), legacy_name, value, &err);
> +        object_property_set_str(OBJECT(dev), value, legacy_name, &err);
>      } else {
> -        object_property_set_str(OBJECT(dev), name, value, &err);
> +        object_property_set_str(OBJECT(dev), value, name, &err);
>      }

Bugfix?

>      g_free(legacy_name);
>  

Andreas
Paolo Bonzini Feb. 3, 2012, 8:11 a.m. UTC | #2 
On 02/02/2012 11:38 PM, Andreas Färber wrote:
> Am 02.02.2012 17:45, schrieb Paolo Bonzini:
>> In some cases, a legacy property does need a special print method
>> but not a special parse method.  In this case, we can reuse the get/set
>> from the static (non-legacy) property.
>>
>> If neither parse nor print is needed, though, do not register the
>> legacy property at all.  The previous patch ensures that the right
>> fallback will be used.
>>
>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>> ---
>>   hw/qdev-monitor.c    |    5 ++---
>>   hw/qdev-properties.c |    6 +++---
>>   hw/qdev.c            |   11 +++++++----
>>   3 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
>> index 64505b4..e21bd50 100644
>> --- a/hw/qdev-monitor.c
>> +++ b/hw/qdev-monitor.c
>> @@ -489,8 +489,8 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
>>   {
>>       if (!props)
>>           return;
>> -    while (props->name) {
>> -        Error *err;
>> +    for (; props->name; props++) {
>> +        Error *err = NULL;
>
> Do either of these fix a bug? Should be mentioned or avoided. [...]
>
> Bugfix?

Yes, I squashed these in the wrong patch, thanks for catching it.

Paolo
diff mbox

Patch

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 64505b4..e21bd50 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -489,8 +489,8 @@  static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
 {
     if (!props)
         return;
-    while (props->name) {
-        Error *err;
+    for (; props->name; props++) {
+        Error *err = NULL;
         char *value;
         char *legacy_name = g_strdup_printf("legacy-%s", props->name);
         if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
@@ -507,7 +507,6 @@  static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
         qdev_printf("%s-prop: %s = %s\n", prefix, props->name,
                     value && *value ? value : "<null>");
         g_free(value);
-        props++;
     }
 }
 
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 7c41140..16f9b22 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1025,13 +1025,13 @@  void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
 int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
 {
     char *legacy_name;
-    Error *err;
+    Error *err = NULL;
 
     legacy_name = g_strdup_printf("legacy-%s", name);
     if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
-        object_property_set_str(OBJECT(dev), legacy_name, value, &err);
+        object_property_set_str(OBJECT(dev), value, legacy_name, &err);
     } else {
-        object_property_set_str(OBJECT(dev), name, value, &err);
+        object_property_set_str(OBJECT(dev), value, name, &err);
     }
     g_free(legacy_name);
 
diff --git a/hw/qdev.c b/hw/qdev.c
index a731e41..660ee38 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -550,21 +550,24 @@  static void qdev_set_legacy_property(Object *obj, Visitor *v, void *opaque,
  * 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 always processed as strings.  The format of the string
- * depends on the property type.
+ * Legacy properties are string versions of other OOM properties.  The format
+ * of the string depends on the property type.
  */
 void qdev_property_add_legacy(DeviceState *dev, Property *prop,
                               Error **errp)
 {
     gchar *name, *type;
 
+    if (!prop->info->print && !prop->info->parse) {
+        return;
+    }
     name = g_strdup_printf("legacy-%s", prop->name);
     type = g_strdup_printf("legacy<%s>",
                            prop->info->legacy_name ?: prop->info->name);
 
     object_property_add(OBJECT(dev), name, type,
-                        prop->info->print ? qdev_get_legacy_property : NULL,
-                        prop->info->parse ? qdev_set_legacy_property : NULL,
+                        prop->info->print ? qdev_get_legacy_property : prop->info->get,
+                        prop->info->parse ? qdev_set_legacy_property : prop->info->set,
                         NULL,
                         prop, errp);