From: Paolo Bonzini <pbonzini@redhat.com>
To: Roman Kiryanov <rkir@google.com>,
Daniel Berrange <berrange@redhat.com>,
qemu-devel <qemu-devel@nongnu.org>
Cc: JP Cottin <jpcottin@google.com>, Erwin Jansen <jansene@google.com>
Subject: Re: QEMU headers in C++
Date: Thu, 2 May 2024 17:19:05 +0200 [thread overview]
Message-ID: <4ead608e-7ab6-44b6-8712-fcf2e7ce6f51@redhat.com> (raw)
In-Reply-To: </a>
On 5/2/24 10:02, Daniel P. Berrangé wrote:
>> Hi QEMU,
>>
>> I work in Android Studio Emulator and we would like to develop devices
>> in C++. Unfortunately, QEMU headers cannot be used with C++ as is
>> (e.g. they use C++ keywords as variable names or implicitly cast void*
>> to T*).
>
> NB, in recent past QEMU explicitly eliminated almost[1] all C++ code from
> the tree, because the consensus was to be exlcusively a C project.
>
>> Will QEMU be open to accept patches from us to make QEMU headers C++
>> compatible?
>
> Personally I think that'd be a retrograde step. Any downstream development
> fork that made use of that facility would be not be able to feed changes
> / additions back into upstream QEMU codebase at a later date, without QEMU
> accepting C++ code once again.
That would have to depend on what the C++ code would do. For example,
if there is a specific library that we find useful and is written in
C++, it may be useful to accept C++ code in order to share code with
other open source projects. I agree that the chances would be small though.
Anyway, just out of curiosity I tried to see what it would take to
compile edu.c as C++ code, which I think lets us give a more informed
answer.
There were a bunch of conflicts with C++ keyword, especially "new",
"class" and "typename". Given how common "klass" is already in the QEMU
code, it would be acceptable to replace "class" with "klass" and
"typename" with "type_name" or "qom_typename".
"new" is common in .c files but rare in headers; there is one occurrence
in include/hw/pci/pci.h, and some in include/qemu/atomic.h which is a
problem anyway (see below). In my opinion, changing
-bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
+bool pci_intx_route_changed(PCIINTxRoute *from, PCIINTxRoute *to);
would not be a huge deal.
In some cases, some of the changes would be good to have anyway, for
example replacing g_try_malloc0 with g_try_new0 (which, as a side
effect, eliminates a C++ error for casting void *) as you have in
include/qemu/bitmap.h. This is a patch that I would accept right away
(and I have just sent it to qemu-trivial in fact).
The big things that are left are:
- uses of typeof, _Generic, __builtin_types_compatible_p and
__builtin_choose_expr, for example in typeof_strip_qual() and
QEMU_MAKE_LOCKABLE
- QemuLockable also has issues due to compound literals, IIRC
Both of these are really a problem only because thread.h and lockable.h
are included almost everywhere (e.g. via ratelimit.h -> blockjob.h ->
block-common.h, or coroutine.h -> block-io.h). Patches to reduce these
"rebuild everything" includes would be acceptable and an improvement.
There is no reason for block-common.h to include blockjob.h, for example.
(For lockable.h, Android Simulator could just wrap most of
it---everything except struct QemuLockable, probably?---in #ifndef
__cplusplus. That patch would not be accepted by QEMU but it would be
very small and localized).
So my answer is that we can't guarantee that QEMU is compilable as C++,
but some of the changes are an improvement in general and some are a
wash. I think accepting the latter is a small price for Google working
on upstreaming the changes and contributing the former.
Paolo
> We'll never control what forks can do, and many will never feed back code
> regardless, but IMHO we should be steering external developers in a way
> that keeps open the door for their changes to be merged back upstream.
next parent reply other threads:[~2024-05-02 15:19 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <a href="raw">
[not found] ` </a>
2024-05-02 15:19 ` Paolo Bonzini [this message]
2024-05-02 16:04 ` QEMU headers in C++ Roman Kiryanov
2024-05-02 4:40 Roman Kiryanov
2024-05-02 6:19 ` Thomas Huth
2024-05-02 14:47 ` Warner Losh
2024-05-02 8:02 ` Daniel P. Berrangé
2024-05-02 15:17 ` Peter Maydell
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=4ead608e-7ab6-44b6-8712-fcf2e7ce6f51@redhat.com \
--to=pbonzini@redhat.com \
--cc=berrange@redhat.com \
--cc=jansene@google.com \
--cc=jpcottin@google.com \
--cc=qemu-devel@nongnu.org \
--cc=rkir@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).