Re: [PATCH v9 23/23] plugins: Support C++

[Akihiko Odaki]

On 2023/10/12 1:21, Thomas Huth wrote:
> On 11/10/2023 17.48, Akihiko Odaki wrote:
> > On 2023/10/11 17:51, Daniel P. Berrangé wrote:
> > > On Wed, Oct 11, 2023 at 04:03:09PM +0900, Akihiko Odaki wrote:
> > > > Make qemu-plugin.h consumable for C++ platform.
> > > >
> > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > > ---
> > > >   docs/devel/tcg-plugins.rst |  4 ++++
> > > >   meson.build                |  2 +-
> > > >   include/qemu/qemu-plugin.h |  4 ++++
> > > >   tests/plugin/cc.cc         | 16 ++++++++++++++++
> > > >   tests/plugin/meson.build   |  5 +++++
> > > >   tests/tcg/Makefile.target  |  3 +--
> > > >   6 files changed, 31 insertions(+), 3 deletions(-)
> > > >   create mode 100644 tests/plugin/cc.cc
> > > >
> > > > diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> > > > index c9f8b27590..984d8012e9 100644
> > > > --- a/docs/devel/tcg-plugins.rst
> > > > +++ b/docs/devel/tcg-plugins.rst
> > > > @@ -283,6 +283,10 @@ run::
> > > >     160          1      0
> > > >     135          1      0
> > > > +- contrib/plugins/cc.cc
> > > > +
> > > > +A pure test plugin to ensure that the plugin API is compatible with 
> > > > C++.
> > > > +
> > >
> > > IMHO we don't need to be adding a test just to prove the
> > > existance of the G_BEGIN_DECLS/G_END_DECLS macros in the
> > > plugin header.
> >
> > Strictly speaking, if you include this header file from C++, the code 
> > will be interpreted as C++ instead of C but with C linkage. That 
> > worries me that the header file may get something not allowed in C++ 
> > in the future.
>
> I think it should be enough if you add the G_BEGIN_DECLS/G_END_DECLS 
> macros here. QEMU is a C project, and it was quite difficult to get rid 
> of the C++ code again, so please don't soften the check in meson.build 
> and don't introduce new .cc files.
> If you have some code somewhere that uses C++ for plugins, I think it 
> would be better to add a regression test there instead.

It doesn't sound right to test the upstream header file in a downstream 
project. It will take time until the fix becomes readily usable for the 
downstream even if the downstream finds a bug.

What about adding a check for CONFIG_PLUGIN before enabling C++? This 
will ensure nobody dares introducing C++ code again for code else plugins.