| Message ID | 20200709141327.14631-7-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | misc rc0 fixes (docs, plugins, docker) | expand |
On 7/9/20 4:13 PM, 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> > [r-b provisional given change to g_intern_string] > Reviewed-by: Clement Deschamps <clement.deschamps@greensocs.com> > Reviewed-by: Emilio G. Cota <cota@braap.org> > > --- > v3 > - return a non-freeable const g_intern_string() > - checkpatch cleanups > --- > include/qemu/qemu-plugin.h | 6 ++++++ > plugins/api.c | 20 ++++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index bab8b0d4b3af..c98c18d6b052 100644 > --- a/include/qemu/qemu-plugin.h > +++ b/include/qemu/qemu-plugin.h > @@ -335,6 +335,12 @@ 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. The string is valid for > + * the lifetime of the plugin. > + */ > +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h); > + > 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 bbdc5a4eb46d..4304e63f0cf8 100644 > --- a/plugins/api.c > +++ b/plugins/api.c > @@ -303,6 +303,26 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr *haddr > return 0; > } > > +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h) > +{ > +#ifdef CONFIG_SOFTMMU > + if (h && h->is_io) { > + MemoryRegionSection *mrs = h->v.io.section; > + if (!mrs->mr->name) { > + unsigned long maddr = 0xffffffff & (uintptr_t) mrs->mr; Why not use uint32_t & PRIx32? uint32_t maddr = (uintptr_t) mrs->mr; > + g_autofree char *temp = g_strdup_printf("anon%08lx", maddr); > + return g_intern_string(temp); Isn't this illegal? temp is definitively not const... > + } else { > + return g_intern_string(mrs->mr->name); > + } > + } else { > + return g_intern_string("RAM"); > + } > +#else > + return g_intern_string("Invalid"); > +#endif > +} > + > /* > * Queries to the number and potential maximum number of vCPUs there > * will be. This helps the plugin dimension per-vcpu arrays. >
Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > On 7/9/20 4:13 PM, 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> >> [r-b provisional given change to g_intern_string] >> Reviewed-by: Clement Deschamps <clement.deschamps@greensocs.com> >> Reviewed-by: Emilio G. Cota <cota@braap.org> >> >> --- >> v3 >> - return a non-freeable const g_intern_string() >> - checkpatch cleanups >> --- >> include/qemu/qemu-plugin.h | 6 ++++++ >> plugins/api.c | 20 ++++++++++++++++++++ >> 2 files changed, 26 insertions(+) >> >> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h >> index bab8b0d4b3af..c98c18d6b052 100644 >> --- a/include/qemu/qemu-plugin.h >> +++ b/include/qemu/qemu-plugin.h >> @@ -335,6 +335,12 @@ 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. The string is valid for >> + * the lifetime of the plugin. >> + */ >> +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h); >> + >> 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 bbdc5a4eb46d..4304e63f0cf8 100644 >> --- a/plugins/api.c >> +++ b/plugins/api.c >> @@ -303,6 +303,26 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr *haddr >> return 0; >> } >> >> +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h) >> +{ >> +#ifdef CONFIG_SOFTMMU >> + if (h && h->is_io) { >> + MemoryRegionSection *mrs = h->v.io.section; >> + if (!mrs->mr->name) { >> + unsigned long maddr = 0xffffffff & (uintptr_t) mrs->mr; > > Why not use uint32_t & PRIx32? > > uint32_t maddr = (uintptr_t) mrs->mr; > >> + g_autofree char *temp = g_strdup_printf("anon%08lx", maddr); >> + return g_intern_string(temp); > > Isn't this illegal? temp is definitively not const... We don't mess with it, we return a new string which is the canonical form. > >> + } else { >> + return g_intern_string(mrs->mr->name); >> + } >> + } else { >> + return g_intern_string("RAM"); >> + } >> +#else >> + return g_intern_string("Invalid"); >> +#endif >> +} >> + >> /* >> * Queries to the number and potential maximum number of vCPUs there >> * will be. This helps the plugin dimension per-vcpu arrays. >> -- Alex Bennée
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index bab8b0d4b3af..c98c18d6b052 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -335,6 +335,12 @@ 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. The string is valid for + * the lifetime of the plugin. + */ +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h); + 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 bbdc5a4eb46d..4304e63f0cf8 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -303,6 +303,26 @@ uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr *haddr return 0; } +const char *qemu_plugin_hwaddr_device_name(const struct qemu_plugin_hwaddr *h) +{ +#ifdef CONFIG_SOFTMMU + if (h && h->is_io) { + MemoryRegionSection *mrs = h->v.io.section; + if (!mrs->mr->name) { + unsigned long maddr = 0xffffffff & (uintptr_t) mrs->mr; + g_autofree char *temp = g_strdup_printf("anon%08lx", maddr); + return g_intern_string(temp); + } else { + return g_intern_string(mrs->mr->name); + } + } else { + return g_intern_string("RAM"); + } +#else + return g_intern_string("Invalid"); +#endif +} + /* * Queries to the number and potential maximum number of vCPUs there * will be. This helps the plugin dimension per-vcpu arrays.