[Xen-devel] [PATCH] RFC: Make xen's public headers a little friendlier for C++.
Shuffle some struct definitions up to file scope so that they remain
in scope in C++ when they're used again later.
Add an automatic check for similar C++ pitfalls, to be run only when
g++ is available.
RFC because it's not clear whether we want to make any commitments to
have the public headers be C++-friendly.
Explicitly _not_ addressing the use of 'private' in various fields,
since we'd previously decided not to fix that.
Reported-by: Razvan Cojocaru rcojoc...@bitdefender.com
Signed-off-by: Tim Deegan t...@xen.org
Cc: Jan Beulich jbeul...@suse.com
---
.gitignore| 1 +
xen/include/Makefile | 12 +---
xen/include/public/platform.h | 39 ++-
3 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/.gitignore b/.gitignore
index 13ee05b..78958ea 100644
--- a/.gitignore
+++ b/.gitignore
@@ -233,6 +233,7 @@ xen/arch/*/efi/compat.c
xen/arch/*/efi/efi.h
xen/arch/*/efi/runtime.c
xen/include/headers.chk
+xen/include/headers++.chk
xen/include/asm
xen/include/asm-*/asm-offsets.h
xen/include/compat/*
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 94112d1..c361877 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -87,13 +87,19 @@ compat/xlat.h: $(addprefix compat/.xlat/,$(xlat-y)) Makefile
ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
-all: headers.chk
+all: headers.chk headers++.chk
-headers.chk: $(filter-out public/arch-% public/%ctl.h public/xsm/%
public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) Makefile
+PUBLIC_HEADERS_TO_CHECK := $(filter-out public/arch-% public/%ctl.h
public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h)
$(public-y))
+
+headers.chk: $(PUBLIC_HEADERS_TO_CHECK) Makefile
for i in $(filter %.h,$^); do $(CC) -ansi -include stdint.h -Wall -W
-Werror -S -o /dev/null -x c $$i || exit 1; echo $$i; done $@.new
mv $@.new $@
+headers++.chk: $(PUBLIC_HEADERS_TO_CHECK) Makefile
+ if g++ -v /dev/null; then for i in $(filter %.h,$^); do g++ -ansi
-include stdint.h -Wall -W -Werror -S -o /dev/null -x c++
-Dprivate=private_is_a_keyword_in_c_plus_plus $$i || exit 1; echo $$i; done ;
fi $@.new
+ mv $@.new $@
+
endif
clean::
- rm -rf compat headers.chk
+ rm -rf compat headers.chk headers++.chk
diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
index 3e340b4..dd03447 100644
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -126,6 +126,26 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_platform_quirk_t);
#define XEN_EFI_query_variable_info 9
#define XEN_EFI_query_capsule_capabilities 10
#define XEN_EFI_update_capsule 11
+
+struct xenpf_efi_guid {
+uint32_t data1;
+uint16_t data2;
+uint16_t data3;
+uint8_t data4[8];
+};
+
+struct xenpf_efi_time {
+uint16_t year;
+uint8_t month;
+uint8_t day;
+uint8_t hour;
+uint8_t min;
+uint8_t sec;
+uint32_t ns;
+int16_t tz;
+uint8_t daylight;
+};
+
struct xenpf_efi_runtime_call {
uint32_t function;
/*
@@ -138,17 +158,7 @@ struct xenpf_efi_runtime_call {
union {
#define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x0001
struct {
-struct xenpf_efi_time {
-uint16_t year;
-uint8_t month;
-uint8_t day;
-uint8_t hour;
-uint8_t min;
-uint8_t sec;
-uint32_t ns;
-int16_t tz;
-uint8_t daylight;
-} time;
+struct xenpf_efi_time time;
uint32_t resolution;
uint32_t accuracy;
} get_time;
@@ -170,12 +180,7 @@ struct xenpf_efi_runtime_call {
XEN_GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */
xen_ulong_t size;
XEN_GUEST_HANDLE(void) data;
-struct xenpf_efi_guid {
-uint32_t data1;
-uint16_t data2;
-uint16_t data3;
-uint8_t data4[8];
-} vendor_guid;
+struct xenpf_efi_guid vendor_guid;
} get_variable, set_variable;
struct {
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] RFC: Make xen's public headers a little friendlier for C++.
On 26.02.15 at 16:22, t...@xen.org wrote: At 14:09 + on 26 Feb (1424956161), Jan Beulich wrote: On 26.02.15 at 14:11, t...@xen.org wrote: +headers++.chk: $(PUBLIC_HEADERS_TO_CHECK) Makefile ... I don't think limiting this to a subset of the headers is the right thing here: C++ consumers are (most likely) going to be user space, i.e. tools, and those would want to be able to use those excluded headers. OK. I'll have a look through that list. I presume I'll still want to exclude e.g. the arch/ headers on the grounds that users shouldn't be including them directly (and we won't want cross-arch includes)? Aren't even our tools doing cross-arch includes. I've been trying to remember why they're excluded from the C check, but can't. If I'm extending this to cover more headers than the ANSI-C check does, should I also relax the '-ansi' requirement to, e.g. '-std=gnu++98'? I think that would be reasonable nowadays. Anyone wanting compatibility farther back could contribute a patch... and I don't see how this step is being avoided when there's no C++ compiler there. if g++ isn't on the path, 'g++ -v' fails and we end up with an empty headers++.chk file. The line is so long that I overlooked that if g++ -v at the beginning (having expected some different mechanism, like suppressing the dependency in that case). Sorry for the noise. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] RFC: Make xen's public headers a little friendlier for C++.
At 14:09 + on 26 Feb (1424956161), Jan Beulich wrote: On 26.02.15 at 14:11, t...@xen.org wrote: -headers.chk: $(filter-out public/arch-% public/%ctl.h public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) Makefile +PUBLIC_HEADERS_TO_CHECK := $(filter-out public/arch-% public/%ctl.h public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) + +headers.chk: $(PUBLIC_HEADERS_TO_CHECK) Makefile for i in $(filter %.h,$^); do $(CC) -ansi -include stdint.h -Wall -W -Werror -S -o /dev/null -x c $$i || exit 1; echo $$i; done $@.new mv $@.new $@ +headers++.chk: $(PUBLIC_HEADERS_TO_CHECK) Makefile ... I don't think limiting this to a subset of the headers is the right thing here: C++ consumers are (most likely) going to be user space, i.e. tools, and those would want to be able to use those excluded headers. OK. I'll have a look through that list. I presume I'll still want to exclude e.g. the arch/ headers on the grounds that users shouldn't be including them directly (and we won't want cross-arch includes)? If I'm extending this to cover more headers than the ANSI-C check does, should I also relax the '-ansi' requirement to, e.g. '-std=gnu++98'? + if g++ -v /dev/null; then for i in $(filter %.h,$^); do g++ -ansi -include stdint.h -Wall -W -Werror -S -o /dev/null -x c++ -Dprivate=private_is_a_keyword_in_c_plus_plus $$i || exit 1; echo $$i; done ; fi $@.new You may want to define __XEN_TOOLS__ (and un-define __XEN__) here. OK. I wonder how many more things will break when I do that. :) Also g++ ought to by abstracted to $(CXX) Sure, I'll define up $(CXX) in StdGnu.mk. and I don't see how this step is being avoided when there's no C++ compiler there. if g++ isn't on the path, 'g++ -v' fails and we end up with an empty headers++.chk file. It doesn't deal with a present-but-broken g++ but that way lies autoconf. --- a/xen/include/public/platform.h +++ b/xen/include/public/platform.h These changes went in a few minutes ago. Good-oh; I'll drop them from a v2. Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
