All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Emilio G. Cota" <cota@braap.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: robert.foley@linaro.org, qemu-devel@nongnu.org,
	robhenry@microsoft.com, aaron@os.amperecomputing.com,
	kuhn.chenqun@huawei.com, peter.puhov@linaro.org
Subject: Re: [PATCH v1 7/9] plugins: add API to return a name for a IO device
Date: Tue, 9 Jun 2020 00:09:02 -0400	[thread overview]
Message-ID: <20200609040902.GA3724030@sff> (raw)
In-Reply-To: <87zh9e6kxy.fsf@linaro.org>

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.

(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


  reply	other threads:[~2020-06-09  4:09 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-02 15:46 [PATCH v1 0/9] plugins/next (bug fixes, hwprofile, lockstep) Alex Bennée
2020-06-02 15:46 ` [PATCH v1 1/9] plugins: new lockstep plugin for debugging TCG changes Alex Bennée
2020-06-02 15:46 ` [PATCH v1 2/9] qemu-plugin.h: add missing include <stddef.h> to define size_t Alex Bennée
2020-06-02 15:46 ` [PATCH v1 3/9] scripts/clean-includes: Mark 'qemu/qemu-plugin.h' as special header Alex Bennée
2020-06-02 15:46 ` [PATCH v1 4/9] tests/plugin: correctly honour io_count Alex Bennée
2020-06-02 17:07   ` Philippe Mathieu-Daudé
2020-06-02 15:46 ` [PATCH v1 5/9] cputlb: ensure we re-fill the TLB if it has reset Alex Bennée
2020-06-02 16:34   ` Richard Henderson
2020-06-02 16:56     ` Alex Bennée
2020-06-02 15:46 ` [PATCH v1 6/9] hw/virtio/pci: include vdev name in registered PCI sections Alex Bennée
2020-06-02 15:59   ` Philippe Mathieu-Daudé
2020-06-04 11:35   ` Michael S. Tsirkin
2020-06-02 15:46 ` [PATCH v1 7/9] plugins: add API to return a name for a IO device Alex Bennée
2020-06-02 16:06   ` Clement Deschamps
2020-06-08  3:45   ` Emilio G. Cota
2020-06-08  6:20     ` Philippe Mathieu-Daudé
2020-06-08  8:06     ` Alex Bennée
2020-06-09  4:09       ` Emilio G. Cota [this message]
2020-06-09 11:09         ` Alex Bennée
2020-06-10  2:32           ` Emilio G. Cota
2020-06-02 15:46 ` [PATCH v1 8/9] plugins: new hwprofile plugin Alex Bennée
2020-06-02 19:16   ` Robert Foley
2020-06-03 11:43     ` Alex Bennée
2020-06-03 15:42       ` Robert Foley
2020-06-03 17:26         ` Alex Bennée
2020-06-03 15:48   ` Peter Maydell
2020-06-03 17:23     ` Alex Bennée
2020-06-02 15:46 ` [PATCH v1 9/9] .travis.yml: allow failure for unreliable hosts Alex Bennée
2020-06-03  8:18   ` Philippe Mathieu-Daudé
2020-06-03 12:40     ` Philippe Mathieu-Daudé
2020-06-11 11:20   ` Thomas Huth
2020-06-02 17:03 ` [PATCH v1 0/9] plugins/next (bug fixes, hwprofile, lockstep) no-reply
2020-06-02 19:16 ` no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200609040902.GA3724030@sff \
    --to=cota@braap.org \
    --cc=aaron@os.amperecomputing.com \
    --cc=alex.bennee@linaro.org \
    --cc=kuhn.chenqun@huawei.com \
    --cc=peter.puhov@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=robert.foley@linaro.org \
    --cc=robhenry@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.