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

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

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);
>>   
>