Re: [PATCH 1/1] include/qemu: Provide a C++ compatible version of typeof_strip_qual

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jun 25, 2024 at 11:16:16AM +0100, Peter Maydell wrote:
> On Tue, 25 Jun 2024 at 10:27, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 25 Jun 2024 at 07:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > >
> > > On 25/6/24 08:05, Paolo Bonzini wrote:
> > > >
> > > >
> > > > Il mar 25 giu 2024, 04:32 Roman Kiryanov <rkir@google.com
> > > > <mailto:rkir@google.com>> ha scritto:
> > > >
> > > >     Hi Philippe, thank you for looking.
> > > >
> > > >     On Mon, Jun 24, 2024 at 7:27 PM Philippe Mathieu-Daudé
> > > >     <philmd@linaro.org <mailto:philmd@linaro.org>> wrote:
> > > >      > In particular this patch seems contained well enough
> > > >      > to be carried in forks were C++ _is_ used.
> > > >
> > > >     Will you agree to take #ifdef __cplusplus  and #error to the QEMU side
> > > >     in atomic.h and
> > > >     we will keep atomic.hpp on our side? The error message looks better
> > > >     when atomic.hpp
> > > >     is somewhere near.
> > > >
> > > >
> > > > I think we should also move typeof_strip_qual elsewhere; I will take a
> > > > look. I think there are a couple headers that already have #ifdef
> > > > __cplusplus, but I need to check (no source code around right now).
> > >
> > > $ git grep -l __cplusplus
> > > ebpf/rss.bpf.skeleton.h
> > > include/hw/xtensa/xtensa-isa.h
> > > include/qemu/compiler.h
> > > include/qemu/osdep.h
> > > include/standard-headers/drm/drm_fourcc.h
> > > include/sysemu/os-posix.h
> > > include/sysemu/os-win32.h
> > > linux-headers/linux/stddef.h
> > > qga/vss-win32/requester.h
> >
> > We should delete all of those, they're dead code for us now.
> > We dropped some of the extern-C-block handling in
> > commit d76aa73fad1f6 but that didn't get all of them.
> 
> I was wrong about this -- I had forgotten about the Windows
> Guest Agent code that has to be built with the Windows C++
> compiler -- some of the cpp files in qga/vss-win32/ include
> osdep.h. So the files above break down into:
> 
>  * files imported from third-party projects, or generated
>    by third-party tools:
>     + ebpf/rss.bpf.skeleton.h
>     + include/hw/xtensa/xtensa-isa.h
>     + include/standard-headers/drm/drm_fourcc.h
>     + linux-headers/linux/stddef.h
> 
>  * QEMU include files that we need to include directly
>    or indirectly from the vss-win32 files:
>     + include/qemu/compiler.h
>     + include/qemu/osdep.h
>     + include/sysemu/os-win32.h
>     + include/sysemu/os-posix.h
>     + qga/vss-win32/requester.h
> 
> Maybe we could drop the cplusplus handling from os-posix.h,
> but I guess we're keeping it in parallel with os-win32.h.

The vss-win32 code is specialized & self-contained code. Since
it is inherantly Windows only code, it does not need any platform
portability support which is what osdep.h would ordinarily assist
with.

As an example, if you remove osdep.h from the vss-win32 code,
the following changes appear sufficient to solve the resulting
compile issues.

This could let us remove remaining cplusplus usage from the
common QEMU headers:

diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
index cc72e5ef1b..ed2d8097ee 100644
--- a/qga/vss-win32/provider.cpp
+++ b/qga/vss-win32/provider.cpp
@@ -10,7 +10,6 @@
  * See the COPYING file in the top-level directory.
  */
 
-#include "qemu/osdep.h"
 #include "vss-common.h"
 #include "vss-debug.h"
 #ifdef HAVE_VSS_SDK
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 9884c65e70..e519e6cfd7 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -10,7 +10,6 @@
  * See the COPYING file in the top-level directory.
  */
 
-#include "qemu/osdep.h"
 #include "vss-common.h"
 #include "vss-debug.h"
 #include "requester.h"
diff --git a/qga/vss-win32/vss-common.h b/qga/vss-win32/vss-common.h
index 0e67e7822c..5c6b21ce21 100644
--- a/qga/vss-win32/vss-common.h
+++ b/qga/vss-win32/vss-common.h
@@ -16,6 +16,9 @@
 #define __MIDL_user_allocate_free_DEFINED__
 #include <windows.h>
 #include <shlwapi.h>
+#include <glib.h>
+#include <assert.h>
+#include "config-host.h"
 
 /* Reduce warnings to include vss.h */
 
diff --git a/qga/vss-win32/vss-debug.cpp b/qga/vss-win32/vss-debug.cpp
index 820b1c6667..ec4c2b3093 100644
--- a/qga/vss-win32/vss-debug.cpp
+++ b/qga/vss-win32/vss-debug.cpp
@@ -10,7 +10,6 @@
  * See the COPYING file in the top-level directory.
  */
 
-#include "qemu/osdep.h"
 #include "vss-debug.h"
 #include "vss-common.h"
 
diff --git a/qga/vss-win32/vss-debug.h b/qga/vss-win32/vss-debug.h
index 7800457392..77fd669698 100644
--- a/qga/vss-win32/vss-debug.h
+++ b/qga/vss-win32/vss-debug.h
@@ -10,8 +10,9 @@
  * See the COPYING file in the top-level directory.
  */
 
-#include "qemu/osdep.h"
 #include <vss-handles.h>
+#include <glib.h>
+#include <stdio.h>
 
 #ifndef VSS_DEBUG_H
 #define VSS_DEBUG_H


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|