Re: [PATCH v1 7/9] plugins: add API to return a name for a IO device

[Emilio G. Cota]

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.bennee@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().

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