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

[Alex Bennée]

Emilio G. Cota <cota@braap.org> writes:

> On Mon, Jun 08, 2020 at 09:06:17 +0100, Alex Bennée wrote:
>> Emilio G. Cota <cota@braap.org> writes:
>> > 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().
>> 
>> AFAIK you can actually mix free/g_free because g_free is just a NULL
>> checking wrapper around free.
>
> I was just going with the documentation, but you're right:
>
> https://github.com/GNOME/glib/blob/mainline/glib/gmem.c#L196
>> void
>> g_free (gpointer mem)
>> {
>>   free (mem);
>>   TRACE(GLIB_MEM_FREE((void*) mem));
>> }
>
> The NULL-pointer check is done by free(3), though.
>
>> However ideally I'd be passing a
>> non-freeable const char to the plugin but I didn't want to expose
>> pointers deep inside of QEMU's guts although maybe I'm just being
>> paranoid there given you can easily gdb the combined operation anyway.
>>
>> Perhaps there is a need for a separate memory region where we can store
>> copies of strings we have made for the plugins?
>
> I agree with the idea of not exposing internal pointers to plugins
> (e.g. we don't pass a CPUState *, only an opaque handle) so I'm OK
> with returning a dup'ed string here.

How about a g_intern_string() as a non-freeable const char that can also
be treated as canonical?

>
> (snip)
>> That said in another
>> thread Peter was uncomfortable about exposing this piece of information
>> to plugins. Maybe we should only expose something based on the optional
>> -device foo,id=bar parameter?
>
> I have no opinion on whether exposing this is a good idea. If it turns
> out that it is, please have my
>
> Reviewed-by: Emilio G. Cota <cota@braap.org>
>
> Thanks,
>
>               Emilio


-- 
Alex Bennée