1. Patchew
  2. QEMU
  3. [PATCH 0/2] hw/block/block.c: improve confusing error
  4. View patch

[PATCH 1/2] hw/core/qdev.c: add qdev_get_human_name()

Manos Pitsidianakis posted 2 patches 10 months, 1 week ago
Maintainers: John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Alistair Francis <alistair@alistair23.me>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
There is a newer version of this series
[PATCH 1/2] hw/core/qdev.c: add qdev_get_human_name()
Posted by Manos Pitsidianakis 10 months, 1 week ago 
Add a simple method to return some kind of human readable identifier for
use in error messages.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/core/qdev.c         | 10 ++++++++++
 include/hw/qdev-core.h | 15 +++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 43d863b0c5..499f191826 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -879,6 +879,16 @@ Object *qdev_get_machine(void)
     return dev;
 }
 
+char *qdev_get_human_name(DeviceState *dev)
+{
+    if (!dev) {
+        return g_strdup("");
+    }
+
+    return dev->id ?
+           g_strdup(dev->id) : object_get_canonical_path(OBJECT(dev));
+}
+
 static MachineInitPhase machine_phase;
 
 bool phase_check(MachineInitPhase phase)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 151d968238..a8c742b4a3 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -993,6 +993,21 @@ const char *qdev_fw_name(DeviceState *dev);
 void qdev_assert_realized_properly(void);
 Object *qdev_get_machine(void);
 
+/**
+ * qdev_get_human_name() - Return a human-readable name for a device
+ * @dev: The device
+ *
+ * .. note::
+ *    This function is intended for user friendly error messages.
+ *
+ * Returns: A newly allocated string containing the device id if not null,
+ * else the object canonical path if not null. If @dev is NULL, it returns an
+ * allocated empty string.
+ *
+ * Use g_free() to free it.
+ */
+char *qdev_get_human_name(DeviceState *dev);
+
 /* FIXME: make this a link<> */
 bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp);
 
-- 
γαῖα πυρί μιχθήτω
Re: [PATCH 1/2] hw/core/qdev.c: add qdev_get_human_name()
Posted by Philippe Mathieu-Daudé 10 months, 1 week ago 
Hi Manos,

On 23/1/24 09:09, Manos Pitsidianakis wrote:
> Add a simple method to return some kind of human readable identifier for
> use in error messages.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>   hw/core/qdev.c         | 10 ++++++++++
>   include/hw/qdev-core.h | 15 +++++++++++++++
>   2 files changed, 25 insertions(+)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 43d863b0c5..499f191826 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -879,6 +879,16 @@ Object *qdev_get_machine(void)
>       return dev;
>   }
>   
> +char *qdev_get_human_name(DeviceState *dev)
> +{
> +    if (!dev) {
> +        return g_strdup("");
> +    }
> +
> +    return dev->id ?
> +           g_strdup(dev->id) : object_get_canonical_path(OBJECT(dev));
> +}
> +
>   static MachineInitPhase machine_phase;
>   
>   bool phase_check(MachineInitPhase phase)
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 151d968238..a8c742b4a3 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -993,6 +993,21 @@ const char *qdev_fw_name(DeviceState *dev);
>   void qdev_assert_realized_properly(void);
>   Object *qdev_get_machine(void);
>   
> +/**
> + * qdev_get_human_name() - Return a human-readable name for a device
> + * @dev: The device
> + *
> + * .. note::
> + *    This function is intended for user friendly error messages.
> + *
> + * Returns: A newly allocated string containing the device id if not null,
> + * else the object canonical path if not null. If @dev is NULL, it returns an
> + * allocated empty string.

In which case do we want to call this with NULL?

> + *
> + * Use g_free() to free it.
> + */
> +char *qdev_get_human_name(DeviceState *dev);
> +
>   /* FIXME: make this a link<> */
>   bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp);
>
Re: [PATCH 1/2] hw/core/qdev.c: add qdev_get_human_name()
Posted by Manos Pitsidianakis 10 months, 1 week ago 
On Tue, 23 Jan 2024 10:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>Hi Manos,
>
>On 23/1/24 09:09, Manos Pitsidianakis wrote:
>> Add a simple method to return some kind of human readable identifier for
>> use in error messages.
>> 
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> ---
>>   hw/core/qdev.c         | 10 ++++++++++
>>   include/hw/qdev-core.h | 15 +++++++++++++++
>>   2 files changed, 25 insertions(+)
>> 
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 43d863b0c5..499f191826 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -879,6 +879,16 @@ Object *qdev_get_machine(void)
>>       return dev;
>>   }
>>   
>> +char *qdev_get_human_name(DeviceState *dev)
>> +{
>> +    if (!dev) {
>> +        return g_strdup("");
>> +    }
>> +
>> +    return dev->id ?
>> +           g_strdup(dev->id) : object_get_canonical_path(OBJECT(dev));
>> +}
>> +
>>   static MachineInitPhase machine_phase;
>>   
>>   bool phase_check(MachineInitPhase phase)
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index 151d968238..a8c742b4a3 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -993,6 +993,21 @@ const char *qdev_fw_name(DeviceState *dev);
>>   void qdev_assert_realized_properly(void);
>>   Object *qdev_get_machine(void);
>>   
>> +/**
>> + * qdev_get_human_name() - Return a human-readable name for a device
>> + * @dev: The device
>> + *
>> + * .. note::
>> + *    This function is intended for user friendly error messages.
>> + *
>> + * Returns: A newly allocated string containing the device id if not null,
>> + * else the object canonical path if not null. If @dev is NULL, it returns an
>> + * allocated empty string.
>
>In which case do we want to call this with NULL?

None I could think of, just future-proofing the NULL case.


>> + *
>> + * Use g_free() to free it.
>> + */
>> +char *qdev_get_human_name(DeviceState *dev);
>> +
>>   /* FIXME: make this a link<> */
>>   bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp);
>>   
>
Re: [PATCH 1/2] hw/core/qdev.c: add qdev_get_human_name()
Posted by Philippe Mathieu-Daudé 10 months, 1 week ago 
On 23/1/24 09:15, Manos Pitsidianakis wrote:
> On Tue, 23 Jan 2024 10:13, Philippe Mathieu-Daudé <philmd@linaro.org> 
> wrote:
>> Hi Manos,
>>
>> On 23/1/24 09:09, Manos Pitsidianakis wrote:
>>> Add a simple method to return some kind of human readable identifier for
>>> use in error messages.
>>>
>>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>>> ---
>>>   hw/core/qdev.c         | 10 ++++++++++
>>>   include/hw/qdev-core.h | 15 +++++++++++++++
>>>   2 files changed, 25 insertions(+)
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 43d863b0c5..499f191826 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -879,6 +879,16 @@ Object *qdev_get_machine(void)
>>>       return dev;
>>>   }
>>> +char *qdev_get_human_name(DeviceState *dev)
>>> +{
>>> +    if (!dev) {
>>> +        return g_strdup("");
>>> +    }
>>> +
>>> +    return dev->id ?
>>> +           g_strdup(dev->id) : object_get_canonical_path(OBJECT(dev));
>>> +}
>>> +
>>>   static MachineInitPhase machine_phase;
>>>   bool phase_check(MachineInitPhase phase)
>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>>> index 151d968238..a8c742b4a3 100644
>>> --- a/include/hw/qdev-core.h
>>> +++ b/include/hw/qdev-core.h
>>> @@ -993,6 +993,21 @@ const char *qdev_fw_name(DeviceState *dev);
>>>   void qdev_assert_realized_properly(void);
>>>   Object *qdev_get_machine(void);
>>> +/**
>>> + * qdev_get_human_name() - Return a human-readable name for a device
>>> + * @dev: The device
>>> + *
>>> + * .. note::
>>> + *    This function is intended for user friendly error messages.
>>> + *
>>> + * Returns: A newly allocated string containing the device id if not 
>>> null,
>>> + * else the object canonical path if not null. If @dev is NULL, it 
>>> returns an
>>> + * allocated empty string.
>>
>> In which case do we want to call this with NULL?
> 
> None I could think of, just future-proofing the NULL case.

I'd rather have a raw assert() as future-proof API, avoiding
dubious corner cases :)

> 
>>> + *
>>> + * Use g_free() to free it.
>>> + */
>>> +char *qdev_get_human_name(DeviceState *dev);
>>> +
>>>   /* FIXME: make this a link<> */
>>>   bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error 
>>> **errp);
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.