| Message ID | 20210413160850.240064-3-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,v2,1/3] osdep: include glib-compat.h before other QEMU headers | expand |
On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote: > System headers may include templates if compiled with a C++ compiler, > which cause the compiler to complain if qemu/osdep.h is included > within a C++ source file's 'extern "C"' block. Add > an 'extern "C"' block directly to qemu/osdep.h, so that > system headers can be kept out of it. > > There is a stray declaration early in qemu/osdep.h, which needs > to be special cased. Add a definition in qemu/compiler.h to > make it look nice. > > config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h > are included outside the 'extern "C"' block; that is not > an issue because they consist entirely of preprocessor directives. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > disas/nanomips.cpp | 2 +- > include/qemu/compiler.h | 6 ++++++ > include/qemu/osdep.h | 10 +++++++++- > 3 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp > index 2b09655271..8ddef897f0 100644 > --- a/disas/nanomips.cpp > +++ b/disas/nanomips.cpp > @@ -27,8 +27,8 @@ > * Reference Manual", Revision 01.01, April 27, 2018 > */ > > -extern "C" { > #include "qemu/osdep.h" > +extern "C" { > #include "disas/dis-asm.h" > } disas/arm-a64.c also has an 'extern "C"' block around an include of qemu/osdep.h. Do we need a similar fix for that file, or are we no longer using that bit of code ? > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index cf28bb2bcd..091c45248b 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -11,6 +11,12 @@ > #define QEMU_STATIC_ANALYSIS 1 > #endif > > +#ifdef __cplusplus > +#define QEMU_EXTERN_C extern "C" > +#else > +#define QEMU_EXTERN_C extern > +#endif > + > #define QEMU_NORETURN __attribute__ ((__noreturn__)) > > #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index b67b0a1e8c..3f8785a471 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -57,7 +57,7 @@ > #define daemon qemu_fake_daemon_function > #include <stdlib.h> > #undef daemon > -extern int daemon(int, int); > +QEMU_EXTERN_C int daemon(int, int); > #endif > > #ifdef _WIN32 > @@ -113,6 +113,10 @@ extern int daemon(int, int); > > #include "glib-compat.h" > > +#ifdef __cplusplus > +extern "C" { > +#endif > + > #ifdef _WIN32 > #include "sysemu/os-win32.h" This and os-posix.h both include other system headers. We don't currently have problem, so this is ok as the minimal fix for 6.0, but long term we need more work on this header to further narrow the extern {} block. So assuming my question about disas/arm-a64.c is a non-issue, then Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote: > > System headers may include templates if compiled with a C++ compiler, > > which cause the compiler to complain if qemu/osdep.h is included > > within a C++ source file's 'extern "C"' block. Add > > an 'extern "C"' block directly to qemu/osdep.h, so that > > system headers can be kept out of it. > > > > There is a stray declaration early in qemu/osdep.h, which needs > > to be special cased. Add a definition in qemu/compiler.h to > > make it look nice. > > > > config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h > > are included outside the 'extern "C"' block; that is not > > an issue because they consist entirely of preprocessor directives. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > disas/nanomips.cpp | 2 +- > > include/qemu/compiler.h | 6 ++++++ > > include/qemu/osdep.h | 10 +++++++++- > > 3 files changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp > > index 2b09655271..8ddef897f0 100644 > > --- a/disas/nanomips.cpp > > +++ b/disas/nanomips.cpp > > @@ -27,8 +27,8 @@ > > * Reference Manual", Revision 01.01, April 27, 2018 > > */ > > > > -extern "C" { > > #include "qemu/osdep.h" > > +extern "C" { > > #include "disas/dis-asm.h" > > } > This and os-posix.h both include other system headers. We don't currently > have problem, so this is ok as the minimal fix for 6.0, but long term we > need more work on this header to further narrow the extern {} block. The other path where we can include system headers inside extern "C" is that the code above still has dis-asm.h inside the extern C block, but dis-asm.h includes qemu/bswap.h (midway down the file!) and bswap.h in turn includes some system headers. thanks -- PMM
On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote: > > #ifdef _WIN32 > > #include "sysemu/os-win32.h" > > This and os-posix.h both include other system headers. We don't currently > have problem, so this is ok as the minimal fix for 6.0, but long term we > need more work on this header to further narrow the extern {} block. Maybe we should just move all the system header includes out of both os-posix.h and os-win32.h ? We already have one header file we've treated that way (sys/wait.h). Alternatively we could leave os-win32.h and os-posix.h outside osdep.h's extern block, and require that they both use an extern block themselves for their declarations. thanks -- PMM
On Wed, Apr 14, 2021 at 07:50:41PM +0100, Peter Maydell wrote: > On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote: > > > #ifdef _WIN32 > > > #include "sysemu/os-win32.h" > > > > This and os-posix.h both include other system headers. We don't currently > > have problem, so this is ok as the minimal fix for 6.0, but long term we > > need more work on this header to further narrow the extern {} block. > > Maybe we should just move all the system header includes out of > both os-posix.h and os-win32.h ? We already have one header file > we've treated that way (sys/wait.h). > > Alternatively we could leave os-win32.h and os-posix.h outside > osdep.h's extern block, and require that they both use an > extern block themselves for their declarations. I'd be inclined towards the latter as I tihnk its reasonable for os-win32/posix.h to want to include system headers. Regards, Daniel
diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp index 2b09655271..8ddef897f0 100644 --- a/disas/nanomips.cpp +++ b/disas/nanomips.cpp @@ -27,8 +27,8 @@ * Reference Manual", Revision 01.01, April 27, 2018 */ -extern "C" { #include "qemu/osdep.h" +extern "C" { #include "disas/dis-asm.h" } diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index cf28bb2bcd..091c45248b 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -11,6 +11,12 @@ #define QEMU_STATIC_ANALYSIS 1 #endif +#ifdef __cplusplus +#define QEMU_EXTERN_C extern "C" +#else +#define QEMU_EXTERN_C extern +#endif + #define QEMU_NORETURN __attribute__ ((__noreturn__)) #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index b67b0a1e8c..3f8785a471 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -57,7 +57,7 @@ #define daemon qemu_fake_daemon_function #include <stdlib.h> #undef daemon -extern int daemon(int, int); +QEMU_EXTERN_C int daemon(int, int); #endif #ifdef _WIN32 @@ -113,6 +113,10 @@ extern int daemon(int, int); #include "glib-compat.h" +#ifdef __cplusplus +extern "C" { +#endif + #ifdef _WIN32 #include "sysemu/os-win32.h" #endif @@ -723,4 +727,8 @@ static inline int platform_does_not_support_system(const char *command) } #endif /* !HAVE_SYSTEM_FUNCTION */ +#ifdef __cplusplus +} +#endif + #endif
System headers may include templates if compiled with a C++ compiler, which cause the compiler to complain if qemu/osdep.h is included within a C++ source file's 'extern "C"' block. Add an 'extern "C"' block directly to qemu/osdep.h, so that system headers can be kept out of it. There is a stray declaration early in qemu/osdep.h, which needs to be special cased. Add a definition in qemu/compiler.h to make it look nice. config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h are included outside the 'extern "C"' block; that is not an issue because they consist entirely of preprocessor directives. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- disas/nanomips.cpp | 2 +- include/qemu/compiler.h | 6 ++++++ include/qemu/osdep.h | 10 +++++++++- 3 files changed, 16 insertions(+), 2 deletions(-)