On 6/8/20 5:45 AM, Emilio G. Cota wrote:
> On Tue, Jun 02, 2020 at 16:46:22 +0100, Alex Bennée wrote:
>> This may well end up being anonymous but it should always be unique.
>>
>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>> ---
>> include/qemu/qemu-plugin.h | 5 +++++
>> plugins/api.c | 18 ++++++++++++++++++
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index bab8b0d4b3a..43c6a9e857f 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -335,6 +335,11 @@ struct qemu_plugin_hwaddr
>> *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
>> bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
>> uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr
>> *haddr);
>>
>> +/*
>> + * Returns a string representing the device. Plugin must free() it
>> + */
>> +char * qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr
>> *haddr);
>> +
>> typedef void
>> (*qemu_plugin_vcpu_mem_cb_t)(unsigned int vcpu_index,
>> qemu_plugin_meminfo_t info, uint64_t vaddr,
>> diff --git a/plugins/api.c b/plugins/api.c
>> index bbdc5a4eb46..3c73de8c1c2 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -303,6 +303,24 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct
>> qemu_plugin_hwaddr *haddr
>> return 0;
>> }
>>
>> +char * qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr
>> *haddr)
>> +{
>> +#ifdef CONFIG_SOFTMMU
>> + if (haddr && haddr->is_io) {
>> + MemoryRegionSection *mrs = haddr->v.io.section;
>> + if (!mrs->mr->name) {
>> + return g_strdup_printf("anon%08lx", 0xffffffff & (uintptr_t)
>> mrs->mr);
>> + } else {
>> + return g_strdup(mrs->mr->name);
>> + }
>> + } else {
>> + return g_strdup("RAM");
>> + }
>> +#else
>> + return g_strdup("Invalid");
>> +#endif
>> +}
>
> I'd rather use asprintf(3) and strdup(3) here, so that plugins don't
> have to worry about glib, and on the QEMU side we don't have to worry
> about plugins calling free() instead of g_free().
It might make sense, but it should be documented in
include/qemu/qemu-plugin.h or docs/devel/tcg-plugins.rst.
>
> Or given that this doesn't look perf-critical, perhaps an easier way out
> is to wrap the above with:
>
> char *g_str = above();
> char *ret = strdup(g_str);
> g_free(g_str);
free() ;)
> return ret;
>
> Not sure we should NULL-check ret, since I don't know whether
> mrs->mr->name is guaranteed to be non-NULL.
>
> Thanks,
> Emilio
>
