qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"
@ 2021-04-13 11:37 Paolo Bonzini
  2021-04-13 11:37 ` [PATCH 1/2] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Paolo Bonzini @ 2021-04-13 11:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, berrange

qemu/osdep.h is quite special in that, despite being part of QEMU sources,
it is included by C++ source files as well.

disas/nanomips.cpp is doing so within an 'extern "C"' block, which breaks
with latest glib due to the inclusion of templates in glib.h.

These patches implement Daniel Berrangé's idea of pushing the 'extern "C"'
block within glib.h and including system headers (including glib.h,
and in fact QEMU's own glib-compat.h too) *outside* the block.

(CI has not finished running yet, but it seems encouraging).

Paolo

Paolo Bonzini (2):
  osdep: include glib-compat.h before other QEMU headers
  osdep: protect qemu/osdep.h with extern "C"

 disas/nanomips.cpp      |  2 +-
 include/qemu/compiler.h |  6 ++++++
 include/qemu/osdep.h    | 13 +++++++++++--
 3 files changed, 18 insertions(+), 3 deletions(-)

-- 
2.30.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] osdep: include glib-compat.h before other QEMU headers
  2021-04-13 11:37 [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C" Paolo Bonzini
@ 2021-04-13 11:37 ` Paolo Bonzini
  2021-04-13 11:37 ` [PATCH 2/2] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2021-04-13 11:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, berrange

glib-compat.h is sort of like a system header, and it needs to include
system headers (glib.h) that may dislike being included under
'extern "C"'.  Move it right after all system headers and before
all other QEMU headers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/osdep.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ba15be9c56..b67b0a1e8c 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -111,6 +111,8 @@ extern int daemon(int, int);
 #define WEXITSTATUS(x) (x)
 #endif
 
+#include "glib-compat.h"
+
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
 #endif
@@ -123,7 +125,6 @@ extern int daemon(int, int);
 #include <AvailabilityMacros.h>
 #endif
 
-#include "glib-compat.h"
 #include "qemu/typedefs.h"
 
 /*
-- 
2.30.1




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] osdep: protect qemu/osdep.h with extern "C"
  2021-04-13 11:37 [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C" Paolo Bonzini
  2021-04-13 11:37 ` [PATCH 1/2] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
@ 2021-04-13 11:37 ` Paolo Bonzini
  2021-04-13 12:00   ` Peter Maydell
  2021-04-13 11:48 ` [PATCH 0/2] osdep: allow including qemu/osdep.h outside " no-reply
  2021-04-13 15:58 ` Philippe Mathieu-Daudé
  3 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2021-04-13 11:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, berrange

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"
 }
 
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
-- 
2.30.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"
  2021-04-13 11:37 [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C" Paolo Bonzini
  2021-04-13 11:37 ` [PATCH 1/2] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
  2021-04-13 11:37 ` [PATCH 2/2] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
@ 2021-04-13 11:48 ` no-reply
  2021-04-13 15:58 ` Philippe Mathieu-Daudé
  3 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2021-04-13 11:48 UTC (permalink / raw)
  To: pbonzini; +Cc: peter.maydell, berrange, qemu-devel

Patchew URL: https://patchew.org/QEMU/20210413113741.214867-1-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210413113741.214867-1-pbonzini@redhat.com
Subject: [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20210408195534.647895-1-antonkuchin@yandex-team.ru -> patchew/20210408195534.647895-1-antonkuchin@yandex-team.ru
 * [new tag]         patchew/20210413113741.214867-1-pbonzini@redhat.com -> patchew/20210413113741.214867-1-pbonzini@redhat.com
Switched to a new branch 'test'
992fa52 osdep: protect qemu/osdep.h with extern "C"
bc2310f osdep: include glib-compat.h before other QEMU headers

=== OUTPUT BEGIN ===
1/2 Checking commit bc2310f731a5 (osdep: include glib-compat.h before other QEMU headers)
2/2 Checking commit 992fa52da413 (osdep: protect qemu/osdep.h with extern "C")
WARNING: architecture specific defines should be avoided
#51: FILE: include/qemu/compiler.h:14:
+#ifdef __cplusplus

ERROR: storage class should be at the beginning of the declaration
#52: FILE: include/qemu/compiler.h:15:
+#define QEMU_EXTERN_C extern "C"

ERROR: storage class should be at the beginning of the declaration
#54: FILE: include/qemu/compiler.h:17:
+#define QEMU_EXTERN_C extern

WARNING: architecture specific defines should be avoided
#77: FILE: include/qemu/osdep.h:116:
+#ifdef __cplusplus

WARNING: architecture specific defines should be avoided
#88: FILE: include/qemu/osdep.h:730:
+#ifdef __cplusplus

total: 2 errors, 3 warnings, 47 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210413113741.214867-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] osdep: protect qemu/osdep.h with extern "C"
  2021-04-13 11:37 ` [PATCH 2/2] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
@ 2021-04-13 12:00   ` Peter Maydell
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2021-04-13 12:00 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Daniel P. Berrange, QEMU Developers

On Tue, 13 Apr 2021 at 12:37, Paolo Bonzini <pbonzini@redhat.com> 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"
>  }
>
> 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

There are some system header includes in osdep.h below this point
(sys/shm.h and sys/uio.h) -- don't they need to be moved up
to go with the other system includes first ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"
  2021-04-13 11:37 [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C" Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-04-13 11:48 ` [PATCH 0/2] osdep: allow including qemu/osdep.h outside " no-reply
@ 2021-04-13 15:58 ` Philippe Mathieu-Daudé
  2021-04-15 16:21   ` Aleksandar Rikalo
  3 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-13 15:58 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: peter.maydell, Aleksandar Rikalo, berrange, Petar Jovanovic,
	Vince.DelVecchio, Filip Vidojevic

Cc'ing MediaTek reviewers.

On 4/13/21 1:37 PM, Paolo Bonzini wrote:
> qemu/osdep.h is quite special in that, despite being part of QEMU sources,
> it is included by C++ source files as well.
> 
> disas/nanomips.cpp is doing so within an 'extern "C"' block, which breaks
> with latest glib due to the inclusion of templates in glib.h.
> 
> These patches implement Daniel Berrangé's idea of pushing the 'extern "C"'
> block within glib.h and including system headers (including glib.h,
> and in fact QEMU's own glib-compat.h too) *outside* the block.
> 
> (CI has not finished running yet, but it seems encouraging).
> 
> Paolo
> 
> Paolo Bonzini (2):
>   osdep: include glib-compat.h before other QEMU headers
>   osdep: protect qemu/osdep.h with extern "C"
> 
>  disas/nanomips.cpp      |  2 +-
>  include/qemu/compiler.h |  6 ++++++
>  include/qemu/osdep.h    | 13 +++++++++++--
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"
  2021-04-13 15:58 ` Philippe Mathieu-Daudé
@ 2021-04-15 16:21   ` Aleksandar Rikalo
  0 siblings, 0 replies; 7+ messages in thread
From: Aleksandar Rikalo @ 2021-04-15 16:21 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: peter.maydell, berrange, Philippe Mathieu-Daudé,
	Petar Jovanovic, Vince.DelVecchio, Filip Vidojevic

[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]

Hi Paolo,

Can you specify how to reproduce the issue ? We need more details about environment.

In my case, everything seems to work fine for the newest version of glib (2.68).

Thank you,
Aleksandar

> qemu/osdep.h is quite special in that, despite being part of QEMU sources,
> it is included by C++ source files as well.
>
> disas/nanomips.cpp is doing so within an 'extern "C"' block, which breaks
> with latest glib due to the inclusion of templates in glib.h.
>
> These patches implement Daniel Berrangé's idea of pushing the 'extern "C"'
> block within glib.h and including system headers (including glib.h,
> and in fact QEMU's own glib-compat.h too) *outside* the block.
>
> (CI has not finished running yet, but it seems encouraging).
>
> Paolo
>
> Paolo Bonzini (2):
>   osdep: include glib-compat.h before other QEMU headers
>   osdep: protect qemu/osdep.h with extern "C"
>
>  disas/nanomips.cpp      |  2 +-
>  include/qemu/compiler.h |  6 ++++++
>  include/qemu/osdep.h    | 13 +++++++++++--
>  3 files changed, 18 insertions(+), 3 deletions(-)
>
> --
> 2.30.1
________________________________
From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> on behalf of Philippe Mathieu-Daudé <f4bug@amsat.org>
Sent: Tuesday, April 13, 2021 5:58 PM
To: Paolo Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org <qemu-devel@nongnu.org>
Cc: peter.maydell@linaro.org <peter.maydell@linaro.org>; berrange@redhat.com <berrange@redhat.com>; Aleksandar Rikalo <Aleksandar.Rikalo@syrmia.com>; Vince.DelVecchio@mediatek.com <Vince.DelVecchio@mediatek.com>; Petar Jovanovic <petar.jovanovic@syrmia.com>; Filip Vidojevic <Filip.Vidojevic@Syrmia.com>
Subject: Re: [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"

Cc'ing MediaTek reviewers.

On 4/13/21 1:37 PM, Paolo Bonzini wrote:
> qemu/osdep.h is quite special in that, despite being part of QEMU sources,
> it is included by C++ source files as well.
>
> disas/nanomips.cpp is doing so within an 'extern "C"' block, which breaks
> with latest glib due to the inclusion of templates in glib.h.
>
> These patches implement Daniel Berrangé's idea of pushing the 'extern "C"'
> block within glib.h and including system headers (including glib.h,
> and in fact QEMU's own glib-compat.h too) *outside* the block.
>
> (CI has not finished running yet, but it seems encouraging).
>
> Paolo
>
> Paolo Bonzini (2):
>   osdep: include glib-compat.h before other QEMU headers
>   osdep: protect qemu/osdep.h with extern "C"
>
>  disas/nanomips.cpp      |  2 +-
>  include/qemu/compiler.h |  6 ++++++
>  include/qemu/osdep.h    | 13 +++++++++++--
>  3 files changed, 18 insertions(+), 3 deletions(-)
>


[-- Attachment #2: Type: text/html, Size: 5292 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-15 17:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 11:37 [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C" Paolo Bonzini
2021-04-13 11:37 ` [PATCH 1/2] osdep: include glib-compat.h before other QEMU headers Paolo Bonzini
2021-04-13 11:37 ` [PATCH 2/2] osdep: protect qemu/osdep.h with extern "C" Paolo Bonzini
2021-04-13 12:00   ` Peter Maydell
2021-04-13 11:48 ` [PATCH 0/2] osdep: allow including qemu/osdep.h outside " no-reply
2021-04-13 15:58 ` Philippe Mathieu-Daudé
2021-04-15 16:21   ` Aleksandar Rikalo

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 NNTP newsgroup(s).