[PATCH 1/3] qdev: Change PropertyInfo method print() to return malloc'ed string

Markus Armbruster posted 3 patches 3 weeks, 2 days ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
[PATCH 1/3] qdev: Change PropertyInfo method print() to return malloc'ed string
Posted by Markus Armbruster 3 weeks, 2 days ago
Simpler (more so after the next commit), and no risk of truncation
because the caller's buffer is too small.  Performance doesn't matter;
the method is only used for "info qdev".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/qdev-properties.h     | 2 +-
 hw/core/qdev-properties-system.c | 7 +++----
 hw/core/qdev-properties.c        | 9 ++++-----
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 0197aa4995..60b8133009 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -34,7 +34,7 @@ struct PropertyInfo {
     const char *description;
     const QEnumLookup *enum_table;
     bool realized_set_allowed; /* allow setting property on realized device */
-    int (*print)(Object *obj, const Property *prop, char *dest, size_t len);
+    char *(*print)(Object *obj, const Property *prop);
     void (*set_default_value)(ObjectProperty *op, const Property *prop);
     ObjectProperty *(*create)(ObjectClass *oc, const char *name,
                               const Property *prop);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 1f810b7ddf..4e1afaac82 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -865,15 +865,14 @@ out:
     visit_end_alternate(v, (void **) &alt);
 }
 
-static int print_pci_devfn(Object *obj, const Property *prop, char *dest,
-                           size_t len)
+static char *print_pci_devfn(Object *obj, const Property *prop)
 {
     int32_t *ptr = object_field_prop_ptr(obj, prop);
 
     if (*ptr == -1) {
-        return snprintf(dest, len, "<unset>");
+        return g_strdup("<unset>");
     } else {
-        return snprintf(dest, len, "%02x.%x", *ptr >> 3, *ptr & 7);
+        return g_strdup_printf("%02x.%x", *ptr >> 3, *ptr & 7);
     }
 }
 
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index b7e8a89ba5..422a486969 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1117,12 +1117,11 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v,
                                      Error **errp)
 {
     const Property *prop = opaque;
+    char *s;
 
-    char buffer[1024];
-    char *ptr = buffer;
-
-    prop->info->print(obj, prop, buffer, sizeof(buffer));
-    visit_type_str(v, name, &ptr, errp);
+    s = prop->info->print(obj, prop);
+    visit_type_str(v, name, &s, errp);
+    g_free(s);
 }
 
 /**
-- 
2.49.0
Re: [PATCH 1/3] qdev: Change PropertyInfo method print() to return malloc'ed string
Posted by Marc-André Lureau 3 weeks, 2 days ago
Hi

On Wed, Oct 22, 2025 at 2:15 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Simpler (more so after the next commit), and no risk of truncation
> because the caller's buffer is too small.  Performance doesn't matter;
> the method is only used for "info qdev".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/hw/qdev-properties.h     | 2 +-
>  hw/core/qdev-properties-system.c | 7 +++----
>  hw/core/qdev-properties.c        | 9 ++++-----
>  3 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 0197aa4995..60b8133009 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -34,7 +34,7 @@ struct PropertyInfo {
>      const char *description;
>      const QEnumLookup *enum_table;
>      bool realized_set_allowed; /* allow setting property on realized device */
> -    int (*print)(Object *obj, const Property *prop, char *dest, size_t len);
> +    char *(*print)(Object *obj, const Property *prop);
>      void (*set_default_value)(ObjectProperty *op, const Property *prop);
>      ObjectProperty *(*create)(ObjectClass *oc, const char *name,
>                                const Property *prop);
> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
> index 1f810b7ddf..4e1afaac82 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -865,15 +865,14 @@ out:
>      visit_end_alternate(v, (void **) &alt);
>  }
>
> -static int print_pci_devfn(Object *obj, const Property *prop, char *dest,
> -                           size_t len)
> +static char *print_pci_devfn(Object *obj, const Property *prop)
>  {
>      int32_t *ptr = object_field_prop_ptr(obj, prop);
>
>      if (*ptr == -1) {
> -        return snprintf(dest, len, "<unset>");
> +        return g_strdup("<unset>");
>      } else {
> -        return snprintf(dest, len, "%02x.%x", *ptr >> 3, *ptr & 7);
> +        return g_strdup_printf("%02x.%x", *ptr >> 3, *ptr & 7);
>      }
>  }
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index b7e8a89ba5..422a486969 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1117,12 +1117,11 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v,
>                                       Error **errp)
>  {
>      const Property *prop = opaque;
> +    char *s;

Why not g_autofree ?

>
> -    char buffer[1024];
> -    char *ptr = buffer;
> -
> -    prop->info->print(obj, prop, buffer, sizeof(buffer));
> -    visit_type_str(v, name, &ptr, errp);
> +    s = prop->info->print(obj, prop);
> +    visit_type_str(v, name, &s, errp);
> +    g_free(s);

otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>  }
>
>  /**
> --
> 2.49.0
>
>


-- 
Marc-André Lureau
Re: [PATCH 1/3] qdev: Change PropertyInfo method print() to return malloc'ed string
Posted by Markus Armbruster 3 weeks, 2 days ago
Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Wed, Oct 22, 2025 at 2:15 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Simpler (more so after the next commit), and no risk of truncation
>> because the caller's buffer is too small.  Performance doesn't matter;
>> the method is only used for "info qdev".
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>

[...]

>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index b7e8a89ba5..422a486969 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -1117,12 +1117,11 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v,
>>                                       Error **errp)
>>  {
>>      const Property *prop = opaque;
>> +    char *s;
>
> Why not g_autofree ?

Old habits...  I think it's a wash here, because there's just one path
through the function.

>> -    char buffer[1024];
>> -    char *ptr = buffer;
>> -
>> -    prop->info->print(obj, prop, buffer, sizeof(buffer));
>> -    visit_type_str(v, name, &ptr, errp);
>> +    s = prop->info->print(obj, prop);
>> +    visit_type_str(v, name, &s, errp);
>> +    g_free(s);
>
> otherwise,
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks!