[PATCH 0/6] xen/build: Use system headers

[PATCH 0/6] xen/build: Use system headers

[Andrew Cooper]

Make use of C standard freestanding headers, to avoid bugs in our own local
versions of inttypes.h and booleans.

Andrew Cooper (6):
  xen/build: Allow the use of C freestanding headers
  xen/build: Use the system stdarg.h header
  xen/build: Use the system stdint.h header
  xen/build: Use the system limits.h header
  xen/build: Use the system stddef.h and inttypes.h headers
  xen/build: Use the system stdbool.h header

 xen/Rules.mk                             |   2 +-
 xen/arch/arm/early_printk.c              |   1 -
 xen/arch/arm/p2m.c                       |   1 -
 xen/arch/arm/platforms/xgene-storm.c     |   1 -
 xen/arch/arm/traps.c                     |   1 -
 xen/arch/x86/acpi/cpu_idle.c             |   2 +-
 xen/arch/x86/genapic/probe.c             |   3 +-
 xen/arch/x86/srat.c                      |   1 -
 xen/common/device_tree.c                 |   1 -
 xen/common/lib.c                         |   1 -
 xen/crypto/vmac.c                        |   1 -
 xen/include/asm-arm/platforms/vexpress.h |   2 +-
 xen/include/asm-arm/types.h              |   9 --
 xen/include/asm-x86/types.h              |   9 --
 xen/include/xen/console.h                |   2 +-
 xen/include/xen/device_tree.h            |   1 -
 xen/include/xen/inttypes.h               | 248 -------------------------------
 xen/include/xen/lib.h                    |   2 -
 xen/include/xen/libelf.h                 |  11 +-
 xen/include/xen/stdarg.h                 |  10 --
 xen/include/xen/stdbool.h                |   9 --
 xen/include/xen/types.h                  |  30 ++--
 xen/include/xen/xenoprof.h               |   2 +-
 23 files changed, 19 insertions(+), 331 deletions(-)
 delete mode 100644 xen/include/xen/inttypes.h
 delete mode 100644 xen/include/xen/stdarg.h
 delete mode 100644 xen/include/xen/stdbool.h

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Andrew Cooper]

The C standard defines two types of conforming implementation; hosted and
freestanding.  A subset of the standard headers are the freestanding headers,
requiring no library support at all to use, and therefore usable by Xen.

Unfortunately, -nostdinc is an overly large switch, and there is no
alternative to only permit inclusion of the freestanding headers.  Removing it
is unfortunate, as we lose the protection it offers, but anyone who does try
to use other parts of the standard library will still fail to link.

While altering this area, replace -fno-builtin with -ffreestanding, which
implies -fno-builtin, but also sets __STDC_HOSTED__ to 0.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/Rules.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index ebe1dc0..e0b7ae6 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -46,7 +46,7 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_X86)   += $(BASEDIR)/crypto/built_in.o
 
-CFLAGS += -nostdinc -fno-builtin -fno-common
+CFLAGS += -ffreestanding -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
 CFLAGS += '-D__OBJECT_FILE__="$@"'
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 2/6] xen/build: Use the system stdarg.h header

[Andrew Cooper]

The C spec identifies stdarg.h as freestanding, and available for use in
non-hosted environments, such as Xen.

This replaces the in-tree xen/stdarg.h, which is completely tied to GCC.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/arm/early_printk.c |  1 -
 xen/common/device_tree.c    |  1 -
 xen/include/xen/lib.h       |  1 -
 xen/include/xen/stdarg.h    | 10 ----------
 xen/include/xen/types.h     |  3 +++
 5 files changed, 3 insertions(+), 13 deletions(-)
 delete mode 100644 xen/include/xen/stdarg.h

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index c85db69..199bcf1 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -11,7 +11,6 @@
 #include <xen/config.h>
 #include <xen/init.h>
 #include <xen/lib.h>
-#include <xen/stdarg.h>
 #include <xen/string.h>
 #include <xen/early_printk.h>
 
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index b39c8ca..b686a62 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -19,7 +19,6 @@
 #include <xen/lib.h>
 #include <xen/libfdt/libfdt.h>
 #include <xen/mm.h>
-#include <xen/stdarg.h>
 #include <xen/string.h>
 #include <xen/cpumask.h>
 #include <xen/ctype.h>
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 1c652bb..66f1d04 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -2,7 +2,6 @@
 #define __LIB_H__
 
 #include <xen/inttypes.h>
-#include <xen/stdarg.h>
 #include <xen/types.h>
 #include <xen/xmalloc.h>
 #include <xen/string.h>
diff --git a/xen/include/xen/stdarg.h b/xen/include/xen/stdarg.h
deleted file mode 100644
index 29249a1..0000000
--- a/xen/include/xen/stdarg.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef __XEN_STDARG_H__
-#define __XEN_STDARG_H__
-
-typedef __builtin_va_list va_list;
-#define va_copy(dest, src)    __builtin_va_copy((dest), (src))
-#define va_start(ap, last)    __builtin_va_start((ap), (last))
-#define va_end(ap)            __builtin_va_end(ap)
-#define va_arg                __builtin_va_arg
-
-#endif /* __XEN_STDARG_H__ */
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index 8596ded..384a02f 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -1,6 +1,9 @@
 #ifndef __TYPES_H__
 #define __TYPES_H__
 
+/* Use the C freestanding headers. */
+#include <stdarg.h>
+
 #include <asm/types.h>
 
 #define BITS_TO_LONGS(bits) \
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 3/6] xen/build: Use the system stdint.h header

[Andrew Cooper]

The C spec identifies stdint.h as freestanding, and available for use in
non-hosted environments, such as Xen.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/crypto/vmac.c       |  1 -
 xen/include/xen/types.h | 12 +-----------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/xen/crypto/vmac.c b/xen/crypto/vmac.c
index f3f2743..db555b3 100644
--- a/xen/crypto/vmac.c
+++ b/xen/crypto/vmac.c
@@ -12,7 +12,6 @@
 #include <xen/types.h>
 #include <xen/lib.h>
 #include <crypto/vmac.h>
-#define UINT64_C(x)  x##ULL
 /* end for Xen */
 
 /* Enable code tuned for 64-bit registers; otherwise tuned for 32-bit */
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index 384a02f..1ff534d 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -3,6 +3,7 @@
 
 /* Use the C freestanding headers. */
 #include <stdarg.h>
+#include <stdint.h>
 
 #include <asm/types.h>
 
@@ -34,21 +35,10 @@ typedef unsigned short          ushort;
 typedef unsigned int            uint;
 typedef unsigned long           ulong;
 
-typedef         __u8            uint8_t;
 typedef         __u8            u_int8_t;
-typedef         __s8            int8_t;
-
-typedef         __u16           uint16_t;
 typedef         __u16           u_int16_t;
-typedef         __s16           int16_t;
-
-typedef         __u32           uint32_t;
 typedef         __u32           u_int32_t;
-typedef         __s32           int32_t;
-
-typedef         __u64           uint64_t;
 typedef         __u64           u_int64_t;
-typedef         __s64           int64_t;
 
 struct domain;
 struct vcpu;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 4/6] xen/build: Use the system limits.h header

[Andrew Cooper]

The C spec identifies limits.h as freestanding, and available for use in
non-hosted environments, such as Xen.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/lib.c        | 1 -
 xen/include/xen/types.h | 8 +-------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/xen/common/lib.c b/xen/common/lib.c
index ae0bbb3..b7df417 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -102,7 +102,6 @@ union uu {
  * These are used for shifting, and also below for halfword extraction
  * and assembly.
  */
-#define CHAR_BIT        8               /* number of bits in a char */
 #define QUAD_BITS       (sizeof(s64) * CHAR_BIT)
 #define LONG_BITS       (sizeof(long) * CHAR_BIT)
 #define HALF_BITS       (sizeof(long) * CHAR_BIT / 2)
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index 1ff534d..a222c6e 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -2,6 +2,7 @@
 #define __TYPES_H__
 
 /* Use the C freestanding headers. */
+#include <limits.h>
 #include <stdarg.h>
 #include <stdint.h>
 
@@ -16,13 +17,6 @@
 #define NULL ((void*)0)
 #endif
 
-#define INT_MAX         ((int)(~0U>>1))
-#define INT_MIN         (-INT_MAX - 1)
-#define UINT_MAX        (~0U)
-#define LONG_MAX        ((long)(~0UL>>1))
-#define LONG_MIN        (-LONG_MAX - 1)
-#define ULONG_MAX       (~0UL)
-
 /* bsd */
 typedef unsigned char           u_char;
 typedef unsigned short          u_short;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 5/6] xen/build: Use the system stddef.h and inttypes.h headers

[Andrew Cooper]

These two headers must be changed in unison, because as noted in c/s 65808a8e
"libelf: check all pointer accesses", PRIxPTR is broken in Xen's local copy of
inttypes.h in 32bit builds.

This is precisely why we shouldn't have our own local copies of the standard
header files.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/srat.c                      |   1 -
 xen/include/asm-arm/platforms/vexpress.h |   2 +-
 xen/include/asm-arm/types.h              |   5 -
 xen/include/asm-x86/types.h              |   5 -
 xen/include/xen/console.h                |   2 +-
 xen/include/xen/inttypes.h               | 248 -------------------------------
 xen/include/xen/lib.h                    |   1 -
 xen/include/xen/libelf.h                 |  10 +-
 xen/include/xen/types.h                  |   4 +-
 xen/include/xen/xenoprof.h               |   2 +-
 10 files changed, 6 insertions(+), 274 deletions(-)
 delete mode 100644 xen/include/xen/inttypes.h

diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index d86783e..b8cf46a 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -13,7 +13,6 @@
 
 #include <xen/init.h>
 #include <xen/mm.h>
-#include <xen/inttypes.h>
 #include <xen/nodemask.h>
 #include <xen/acpi.h>
 #include <xen/numa.h>
diff --git a/xen/include/asm-arm/platforms/vexpress.h b/xen/include/asm-arm/platforms/vexpress.h
index 5cf3aba..d599530 100644
--- a/xen/include/asm-arm/platforms/vexpress.h
+++ b/xen/include/asm-arm/platforms/vexpress.h
@@ -27,7 +27,7 @@
 #define SP810_ADDRESS 0x1C020000
 
 #ifndef __ASSEMBLY__
-#include <xen/inttypes.h>
+#include <xen/types.h>
 
 int vexpress_syscfg(int write, int function, int device, uint32_t *data);
 #endif
diff --git a/xen/include/asm-arm/types.h b/xen/include/asm-arm/types.h
index 09e5455..fc43fb4 100644
--- a/xen/include/asm-arm/types.h
+++ b/xen/include/asm-arm/types.h
@@ -55,11 +55,6 @@ typedef u64 register_t;
 #define PRIregister "lx"
 #endif
 
-#if defined(__SIZE_TYPE__)
-typedef __SIZE_TYPE__ size_t;
-#else
-typedef unsigned long size_t;
-#endif
 typedef signed long ssize_t;
 
 typedef char bool_t;
diff --git a/xen/include/asm-x86/types.h b/xen/include/asm-x86/types.h
index b82fa58..c37b516 100644
--- a/xen/include/asm-x86/types.h
+++ b/xen/include/asm-x86/types.h
@@ -34,11 +34,6 @@ typedef unsigned long paddr_t;
 #define INVALID_PADDR (~0UL)
 #define PRIpaddr "016lx"
 
-#if defined(__SIZE_TYPE__)
-typedef __SIZE_TYPE__ size_t;
-#else
-typedef unsigned long size_t;
-#endif
 typedef signed long ssize_t;
 
 typedef char bool_t;
diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index ea06fd8..243031d 100644
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -7,7 +7,7 @@
 #ifndef __CONSOLE_H__
 #define __CONSOLE_H__
 
-#include <xen/inttypes.h>
+#include <xen/types.h>
 #include <public/xen.h>
 
 struct xen_sysctl_readconsole;
diff --git a/xen/include/xen/inttypes.h b/xen/include/xen/inttypes.h
deleted file mode 100644
index 28c0053..0000000
--- a/xen/include/xen/inttypes.h
+++ /dev/null
@@ -1,248 +0,0 @@
-/* Copyright (C) 1997-2001, 2004 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; If not, see <http://www.gnu.org/licenses/>.  */
-
-/*
- *	ISO C99: 7.8 Format conversion of integer types	<inttypes.h>
- */
-
-#ifndef _XEN_INTTYPES_H
-#define _XEN_INTTYPES_H	1
-
-#include <xen/types.h>
-
-# if BITS_PER_LONG == 64
-#  define __PRI64_PREFIX	"l"
-#  define __PRIPTR_PREFIX	"l"
-# else
-#  define __PRI64_PREFIX	"ll"
-#  define __PRIPTR_PREFIX
-# endif
-
-/* Macros for printing format specifiers.  */
-
-/* Decimal notation.  */
-# define PRId8		"d"
-# define PRId16		"d"
-# define PRId32		"d"
-# define PRId64		__PRI64_PREFIX "d"
-
-# define PRIdLEAST8	"d"
-# define PRIdLEAST16	"d"
-# define PRIdLEAST32	"d"
-# define PRIdLEAST64	__PRI64_PREFIX "d"
-
-# define PRIdFAST8	"d"
-# define PRIdFAST16	__PRIPTR_PREFIX "d"
-# define PRIdFAST32	__PRIPTR_PREFIX "d"
-# define PRIdFAST64	__PRI64_PREFIX "d"
-
-
-# define PRIi8		"i"
-# define PRIi16		"i"
-# define PRIi32		"i"
-# define PRIi64		__PRI64_PREFIX "i"
-
-# define PRIiLEAST8	"i"
-# define PRIiLEAST16	"i"
-# define PRIiLEAST32	"i"
-# define PRIiLEAST64	__PRI64_PREFIX "i"
-
-# define PRIiFAST8	"i"
-# define PRIiFAST16	__PRIPTR_PREFIX "i"
-# define PRIiFAST32	__PRIPTR_PREFIX "i"
-# define PRIiFAST64	__PRI64_PREFIX "i"
-
-/* Octal notation.  */
-# define PRIo8		"o"
-# define PRIo16		"o"
-# define PRIo32		"o"
-# define PRIo64		__PRI64_PREFIX "o"
-
-# define PRIoLEAST8	"o"
-# define PRIoLEAST16	"o"
-# define PRIoLEAST32	"o"
-# define PRIoLEAST64	__PRI64_PREFIX "o"
-
-# define PRIoFAST8	"o"
-# define PRIoFAST16	__PRIPTR_PREFIX "o"
-# define PRIoFAST32	__PRIPTR_PREFIX "o"
-# define PRIoFAST64	__PRI64_PREFIX "o"
-
-/* Unsigned integers.  */
-# define PRIu8		"u"
-# define PRIu16		"u"
-# define PRIu32		"u"
-# define PRIu64		__PRI64_PREFIX "u"
-
-# define PRIuLEAST8	"u"
-# define PRIuLEAST16	"u"
-# define PRIuLEAST32	"u"
-# define PRIuLEAST64	__PRI64_PREFIX "u"
-
-# define PRIuFAST8	"u"
-# define PRIuFAST16	__PRIPTR_PREFIX "u"
-# define PRIuFAST32	__PRIPTR_PREFIX "u"
-# define PRIuFAST64	__PRI64_PREFIX "u"
-
-/* lowercase hexadecimal notation.  */
-# define PRIx8		"x"
-# define PRIx16		"x"
-# define PRIx32		"x"
-# define PRIx64		__PRI64_PREFIX "x"
-
-# define PRIxLEAST8	"x"
-# define PRIxLEAST16	"x"
-# define PRIxLEAST32	"x"
-# define PRIxLEAST64	__PRI64_PREFIX "x"
-
-# define PRIxFAST8	"x"
-# define PRIxFAST16	__PRIPTR_PREFIX "x"
-# define PRIxFAST32	__PRIPTR_PREFIX "x"
-# define PRIxFAST64	__PRI64_PREFIX "x"
-
-/* UPPERCASE hexadecimal notation.  */
-# define PRIX8		"X"
-# define PRIX16		"X"
-# define PRIX32		"X"
-# define PRIX64		__PRI64_PREFIX "X"
-
-# define PRIXLEAST8	"X"
-# define PRIXLEAST16	"X"
-# define PRIXLEAST32	"X"
-# define PRIXLEAST64	__PRI64_PREFIX "X"
-
-# define PRIXFAST8	"X"
-# define PRIXFAST16	__PRIPTR_PREFIX "X"
-# define PRIXFAST32	__PRIPTR_PREFIX "X"
-# define PRIXFAST64	__PRI64_PREFIX "X"
-
-
-/* Macros for printing `intmax_t' and `uintmax_t'.  */
-# define PRIdMAX	__PRI64_PREFIX "d"
-# define PRIiMAX	__PRI64_PREFIX "i"
-# define PRIoMAX	__PRI64_PREFIX "o"
-# define PRIuMAX	__PRI64_PREFIX "u"
-# define PRIxMAX	__PRI64_PREFIX "x"
-# define PRIXMAX	__PRI64_PREFIX "X"
-
-
-/* Macros for printing `intptr_t' and `uintptr_t'.  */
-# define PRIdPTR	__PRIPTR_PREFIX "d"
-# define PRIiPTR	__PRIPTR_PREFIX "i"
-# define PRIoPTR	__PRIPTR_PREFIX "o"
-# define PRIuPTR	__PRIPTR_PREFIX "u"
-# define PRIxPTR	__PRIPTR_PREFIX "x"
-# define PRIXPTR	__PRIPTR_PREFIX "X"
-
-
-/* Macros for scanning format specifiers.  */
-
-/* Signed decimal notation.  */
-# define SCNd8		"hhd"
-# define SCNd16		"hd"
-# define SCNd32		"d"
-# define SCNd64		__PRI64_PREFIX "d"
-
-# define SCNdLEAST8	"hhd"
-# define SCNdLEAST16	"hd"
-# define SCNdLEAST32	"d"
-# define SCNdLEAST64	__PRI64_PREFIX "d"
-
-# define SCNdFAST8	"hhd"
-# define SCNdFAST16	__PRIPTR_PREFIX "d"
-# define SCNdFAST32	__PRIPTR_PREFIX "d"
-# define SCNdFAST64	__PRI64_PREFIX "d"
-
-/* Signed decimal notation.  */
-# define SCNi8		"hhi"
-# define SCNi16		"hi"
-# define SCNi32		"i"
-# define SCNi64		__PRI64_PREFIX "i"
-
-# define SCNiLEAST8	"hhi"
-# define SCNiLEAST16	"hi"
-# define SCNiLEAST32	"i"
-# define SCNiLEAST64	__PRI64_PREFIX "i"
-
-# define SCNiFAST8	"hhi"
-# define SCNiFAST16	__PRIPTR_PREFIX "i"
-# define SCNiFAST32	__PRIPTR_PREFIX "i"
-# define SCNiFAST64	__PRI64_PREFIX "i"
-
-/* Unsigned decimal notation.  */
-# define SCNu8		"hhu"
-# define SCNu16		"hu"
-# define SCNu32		"u"
-# define SCNu64		__PRI64_PREFIX "u"
-
-# define SCNuLEAST8	"hhu"
-# define SCNuLEAST16	"hu"
-# define SCNuLEAST32	"u"
-# define SCNuLEAST64	__PRI64_PREFIX "u"
-
-# define SCNuFAST8	"hhu"
-# define SCNuFAST16	__PRIPTR_PREFIX "u"
-# define SCNuFAST32	__PRIPTR_PREFIX "u"
-# define SCNuFAST64	__PRI64_PREFIX "u"
-
-/* Octal notation.  */
-# define SCNo8		"hho"
-# define SCNo16		"ho"
-# define SCNo32		"o"
-# define SCNo64		__PRI64_PREFIX "o"
-
-# define SCNoLEAST8	"hho"
-# define SCNoLEAST16	"ho"
-# define SCNoLEAST32	"o"
-# define SCNoLEAST64	__PRI64_PREFIX "o"
-
-# define SCNoFAST8	"hho"
-# define SCNoFAST16	__PRIPTR_PREFIX "o"
-# define SCNoFAST32	__PRIPTR_PREFIX "o"
-# define SCNoFAST64	__PRI64_PREFIX "o"
-
-/* Hexadecimal notation.  */
-# define SCNx8		"hhx"
-# define SCNx16		"hx"
-# define SCNx32		"x"
-# define SCNx64		__PRI64_PREFIX "x"
-
-# define SCNxLEAST8	"hhx"
-# define SCNxLEAST16	"hx"
-# define SCNxLEAST32	"x"
-# define SCNxLEAST64	__PRI64_PREFIX "x"
-
-# define SCNxFAST8	"hhx"
-# define SCNxFAST16	__PRIPTR_PREFIX "x"
-# define SCNxFAST32	__PRIPTR_PREFIX "x"
-# define SCNxFAST64	__PRI64_PREFIX "x"
-
-
-/* Macros for scanning `intmax_t' and `uintmax_t'.  */
-# define SCNdMAX	__PRI64_PREFIX "d"
-# define SCNiMAX	__PRI64_PREFIX "i"
-# define SCNoMAX	__PRI64_PREFIX "o"
-# define SCNuMAX	__PRI64_PREFIX "u"
-# define SCNxMAX	__PRI64_PREFIX "x"
-
-/* Macros for scaning `intptr_t' and `uintptr_t'.  */
-# define SCNdPTR	__PRIPTR_PREFIX "d"
-# define SCNiPTR	__PRIPTR_PREFIX "i"
-# define SCNoPTR	__PRIPTR_PREFIX "o"
-# define SCNuPTR	__PRIPTR_PREFIX "u"
-# define SCNxPTR	__PRIPTR_PREFIX "x"
-
-#endif /* _XEN_INTTYPES_H */
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 66f1d04..10c47a9 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -1,7 +1,6 @@
 #ifndef __LIB_H__
 #define __LIB_H__
 
-#include <xen/inttypes.h>
 #include <xen/types.h>
 #include <xen/xmalloc.h>
 #include <xen/string.h>
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 95b5370..d430c83 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -83,15 +83,7 @@ typedef uintptr_t elf_ptrval;
 #define ELF_HANDLE_DECL(structname)          structname##_handle
   /* Provides a type declaration for a HANDLE. */
 
-#ifdef __XEN__
-# define ELF_PRPTRVAL "lx"
-  /*
-   * PRIxPTR is misdefined in xen/include/xen/inttypes.h, on 32-bit,
-   * to "x", when in fact uintptr_t is an unsigned long.
-   */
-#else
-# define ELF_PRPTRVAL PRIxPTR
-#endif
+#define ELF_PRPTRVAL PRIxPTR
   /* printf format a la PRId... for a PTRVAL */
 
 #define ELF_DEFINE_HANDLE(structname)                                   \
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index a222c6e..3ea5941 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -2,8 +2,10 @@
 #define __TYPES_H__
 
 /* Use the C freestanding headers. */
+#include <inttypes.h>
 #include <limits.h>
 #include <stdarg.h>
+#include <stddef.h>
 #include <stdint.h>
 
 #include <asm/types.h>
@@ -44,6 +46,4 @@ typedef __u32 __be32;
 typedef __u64 __le64;
 typedef __u64 __be64;
 
-typedef unsigned long uintptr_t;
-
 #endif /* __TYPES_H__ */
diff --git a/xen/include/xen/xenoprof.h b/xen/include/xen/xenoprof.h
index 8f045ab..7d760ca 100644
--- a/xen/include/xen/xenoprof.h
+++ b/xen/include/xen/xenoprof.h
@@ -10,7 +10,7 @@
 #ifndef __XEN_XENOPROF_H__
 #define __XEN_XENOPROF_H__
 
-#include <xen/inttypes.h>
+#include <xen/types.h>
 #include <public/xenoprof.h>
 #include <asm/xenoprof.h>
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 6/6] xen/build: Use the system stdbool.h header

[Andrew Cooper]

and switch bool_t to being of type _Bool rather than char.

Using bool_t as char causes several subtle problems; first that a bool_t
actually has more than two values, and that (bool_t)0x100 actually has the
value 0 rather than the expected 1, due to truncation.

Making this change reveals two bugs now caught by the compiler.
errata_c6_eoi_workaround() actually makes use of bool_t having more than two
states, while generic_apic_probe() has a integer in the middle of a compound
bool_t assignment.

Finally, it turns out that ARM is mixing and matching bool_t and bool, despite
their different semantics.  This change brings the semantics of bool_t to
match bool, but does not alter the current mix.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/arm/p2m.c                   | 1 -
 xen/arch/arm/platforms/xgene-storm.c | 1 -
 xen/arch/arm/traps.c                 | 1 -
 xen/arch/x86/acpi/cpu_idle.c         | 2 +-
 xen/arch/x86/genapic/probe.c         | 3 ++-
 xen/include/asm-arm/types.h          | 4 ----
 xen/include/asm-x86/types.h          | 4 ----
 xen/include/xen/device_tree.h        | 1 -
 xen/include/xen/libelf.h             | 1 -
 xen/include/xen/stdbool.h            | 9 ---------
 xen/include/xen/types.h              | 5 +++++
 11 files changed, 8 insertions(+), 24 deletions(-)
 delete mode 100644 xen/include/xen/stdbool.h

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 65d8f1a..8308028 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1,7 +1,6 @@
 #include <xen/config.h>
 #include <xen/sched.h>
 #include <xen/lib.h>
-#include <xen/stdbool.h>
 #include <xen/errno.h>
 #include <xen/domain_page.h>
 #include <xen/bitops.h>
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index 70cb655..686b19b 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -20,7 +20,6 @@
 
 #include <xen/config.h>
 #include <asm/platform.h>
-#include <xen/stdbool.h>
 #include <xen/vmap.h>
 #include <xen/device_tree.h>
 #include <asm/io.h>
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 44926ca..c6d6c9f 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -17,7 +17,6 @@
  */
 
 #include <xen/config.h>
-#include <xen/stdbool.h>
 #include <xen/init.h>
 #include <xen/string.h>
 #include <xen/version.h>
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index a21aeed..a9ad201 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -480,7 +480,7 @@ void trace_exit_reason(u32 *irq_traced)
  */
 bool_t errata_c6_eoi_workaround(void)
 {
-    static bool_t fix_needed = -1;
+    static int fix_needed = -1;
 
     if ( unlikely(fix_needed == -1) )
     {
diff --git a/xen/arch/x86/genapic/probe.c b/xen/arch/x86/genapic/probe.c
index a5f2a24..8736013 100644
--- a/xen/arch/x86/genapic/probe.c
+++ b/xen/arch/x86/genapic/probe.c
@@ -56,7 +56,8 @@ custom_param("apic", genapic_apic_force);
 
 void __init generic_apic_probe(void) 
 { 
-	int i, changed;
+	bool_t changed;
+	int i;
 
 	record_boot_APIC_mode();
 
diff --git a/xen/include/asm-arm/types.h b/xen/include/asm-arm/types.h
index fc43fb4..d3a7eb6 100644
--- a/xen/include/asm-arm/types.h
+++ b/xen/include/asm-arm/types.h
@@ -57,10 +57,6 @@ typedef u64 register_t;
 
 typedef signed long ssize_t;
 
-typedef char bool_t;
-#define test_and_set_bool(b)   xchg(&(b), 1)
-#define test_and_clear_bool(b) xchg(&(b), 0)
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ARM_TYPES_H__ */
diff --git a/xen/include/asm-x86/types.h b/xen/include/asm-x86/types.h
index c37b516..eebd374 100644
--- a/xen/include/asm-x86/types.h
+++ b/xen/include/asm-x86/types.h
@@ -36,10 +36,6 @@ typedef unsigned long paddr_t;
 
 typedef signed long ssize_t;
 
-typedef char bool_t;
-#define test_and_set_bool(b)   xchg(&(b), 1)
-#define test_and_clear_bool(b) xchg(&(b), 0)
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* __X86_TYPES_H__ */
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index d7d1b40..3657ac2 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -17,7 +17,6 @@
 #include <xen/init.h>
 #include <xen/string.h>
 #include <xen/types.h>
-#include <xen/stdbool.h>
 #include <xen/list.h>
 
 #define DEVICE_TREE_MAX_DEPTH 16
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index d430c83..9ff4006 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -37,7 +37,6 @@ typedef int elf_negerrnoval; /* 0: ok; -EFOO: error */
 #ifdef __XEN__
 #include <public/elfnote.h>
 #include <public/features.h>
-#include <xen/stdbool.h>
 #include <xen/string.h>
 #else
 #include <xen/elfnote.h>
diff --git a/xen/include/xen/stdbool.h b/xen/include/xen/stdbool.h
deleted file mode 100644
index b0947a6..0000000
--- a/xen/include/xen/stdbool.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef __XEN_STDBOOL_H__
-#define __XEN_STDBOOL_H__
-
-#define bool _Bool
-#define true 1
-#define false 0
-#define __bool_true_false_are_defined   1
-
-#endif /* __XEN_STDBOOL_H__ */
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index 3ea5941..4455f8f 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -5,6 +5,7 @@
 #include <inttypes.h>
 #include <limits.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
 
@@ -46,4 +47,8 @@ typedef __u32 __be32;
 typedef __u64 __le64;
 typedef __u64 __be64;
 
+typedef _Bool bool_t;
+#define test_and_set_bool(b)   xchg(&(b), 1)
+#define test_and_clear_bool(b) xchg(&(b), 0)
+
 #endif /* __TYPES_H__ */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[George Dunlap]

On 22/06/16 12:24, Andrew Cooper wrote:
> The C standard defines two types of conforming implementation; hosted and
> freestanding.  A subset of the standard headers are the freestanding headers,
> requiring no library support at all to use, and therefore usable by Xen.
> 
> Unfortunately, -nostdinc is an overly large switch, and there is no
> alternative to only permit inclusion of the freestanding headers.  Removing it
> is unfortunate, as we lose the protection it offers, but anyone who does try
> to use other parts of the standard library will still fail to link.

As long as there's no chance that random library functions will find
their way into the Xen code, I think this series is OK.

 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xen/build: Use system headers

[Jan Beulich]

>>> On 22.06.16 at 13:24, <andrew.cooper3@citrix.com> wrote:
> Make use of C standard freestanding headers, to avoid bugs in our own local
> versions of inttypes.h and booleans.

While the motivation to do this is certainly a good one, I see possible
problems resulting from this. These are best demonstrated by
compiling a C file containing just

#include <inttypes.h>
#include <limits.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

with (among other relevant options) -MD, and with a cross compiler.
The resulting dependencies are

std.o: std.c /usr/local/x86_64-unknown-linux/sys-include/inttypes.h \
 /usr/local/x86_64-unknown-linux/sys-include/features.h \
 /usr/local/x86_64-unknown-linux/sys-include/sys/cdefs.h \
 /usr/local/x86_64-unknown-linux/sys-include/bits/wordsize.h \
 /usr/local/x86_64-unknown-linux/sys-include/gnu/stubs.h \
 /usr/local/x86_64-unknown-linux/sys-include/gnu/stubs-64.h \
 /build/gcc/5.4.0-x86_64/gcc/include/stdint.h \
 /build/gcc/5.4.0-x86_64/gcc/include/stdint-gcc.h \
 /build/gcc/5.4.0-x86_64/gcc/include-fixed/limits.h \
 /build/gcc/5.4.0-x86_64/gcc/include-fixed/syslimits.h \
 /usr/local/x86_64-unknown-linux/sys-include/limits.h \
 /usr/local/x86_64-unknown-linux/sys-include/bits/posix1_lim.h \
 /usr/local/x86_64-unknown-linux/sys-include/bits/local_lim.h \
 /usr/local/x86_64-unknown-linux/sys-include/linux/limits.h \
 /usr/local/x86_64-unknown-linux/sys-include/bits/posix2_lim.h \
 /build/gcc/5.4.0-x86_64/gcc/include/stdarg.h \
 /build/gcc/5.4.0-x86_64/gcc/include/stdbool.h \
 /build/gcc/5.4.0-x86_64/gcc/include/stddef.h

This tells us that this uses not just compiler provided headers,
but also ones provided by the platform (inttypes.h, limits.h,
plus their descendants). I.e. we would not only become
dependent upon whatever the platform library provides, but we'd
also become dependent on there being a respective header
installed in the first place. While that's quite likely a true
assumption for native builds, I don't think we should assume this
for cross builds.

Additionally, looking through the resulting preprocessed file I also
find

typedef int __gwchar_t;

typedef struct
  {
    long int quot;
    long int rem;
  } imaxdiv_t;

extern intmax_t imaxabs (intmax_t __n) __attribute__ ((__nothrow__)) __attribute__ ((__const__));

extern imaxdiv_t imaxdiv (intmax_t __numer, intmax_t __denom)
      __attribute__ ((__nothrow__)) __attribute__ ((__const__));

extern intmax_t strtoimax (__const char *__restrict __nptr,
      char **__restrict __endptr, int __base) __attribute__ ((__nothrow__));

extern uintmax_t strtoumax (__const char *__restrict __nptr,
       char ** __restrict __endptr, int __base) __attribute__ ((__nothrow__));

extern intmax_t wcstoimax (__const __gwchar_t *__restrict __nptr,
      __gwchar_t **__restrict __endptr, int __base)
     __attribute__ ((__nothrow__));

extern uintmax_t wcstoumax (__const __gwchar_t *__restrict __nptr,
       __gwchar_t ** __restrict __endptr, int __base)
     __attribute__ ((__nothrow__));

typedef struct {
  long long __max_align_ll __attribute__((__aligned__(__alignof__(long long))));
  long double __max_align_ld __attribute__((__aligned__(__alignof__(long double))));
} max_align_t;

none of which we want, I think (and I didn't even try to look at the
set of resulting #define-s). Yes, the function declarations are benign
as using them will result in linking failures, but it's still stuff getting
added to the name space which we don't need or want.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2/6] xen/build: Use the system stdarg.h header

[Jan Beulich]

>>> On 22.06.16 at 13:24, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/include/xen/types.h
> +++ b/xen/include/xen/types.h
> @@ -1,6 +1,9 @@
>  #ifndef __TYPES_H__
>  #define __TYPES_H__
>  
> +/* Use the C freestanding headers. */
> +#include <stdarg.h>

I don't view it as good practice to include headers that aren't really
needed: Why would basically every translation unit pay the (however
small) price of this getting read and parsed? And the description of
the patch also doesn't justify this additional change in any way.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xen/build: Use system headers

[Andrew Cooper]

On 22/06/16 13:26, Jan Beulich wrote:
>>>> On 22.06.16 at 13:24, <andrew.cooper3@citrix.com> wrote:
>> Make use of C standard freestanding headers, to avoid bugs in our own local
>> versions of inttypes.h and booleans.
> While the motivation to do this is certainly a good one, I see possible
> problems resulting from this. These are best demonstrated by
> compiling a C file containing just
>
> #include <inttypes.h>
> #include <limits.h>
> #include <stdarg.h>
> #include <stdbool.h>
> #include <stddef.h>
> #include <stdint.h>
>
> with (among other relevant options) -MD, and with a cross compiler.
> The resulting dependencies are
>
> std.o: std.c /usr/local/x86_64-unknown-linux/sys-include/inttypes.h \
>  /usr/local/x86_64-unknown-linux/sys-include/features.h \
>  /usr/local/x86_64-unknown-linux/sys-include/sys/cdefs.h \
>  /usr/local/x86_64-unknown-linux/sys-include/bits/wordsize.h \
>  /usr/local/x86_64-unknown-linux/sys-include/gnu/stubs.h \
>  /usr/local/x86_64-unknown-linux/sys-include/gnu/stubs-64.h \
>  /build/gcc/5.4.0-x86_64/gcc/include/stdint.h \
>  /build/gcc/5.4.0-x86_64/gcc/include/stdint-gcc.h \
>  /build/gcc/5.4.0-x86_64/gcc/include-fixed/limits.h \
>  /build/gcc/5.4.0-x86_64/gcc/include-fixed/syslimits.h \
>  /usr/local/x86_64-unknown-linux/sys-include/limits.h \
>  /usr/local/x86_64-unknown-linux/sys-include/bits/posix1_lim.h \
>  /usr/local/x86_64-unknown-linux/sys-include/bits/local_lim.h \
>  /usr/local/x86_64-unknown-linux/sys-include/linux/limits.h \
>  /usr/local/x86_64-unknown-linux/sys-include/bits/posix2_lim.h \
>  /build/gcc/5.4.0-x86_64/gcc/include/stdarg.h \
>  /build/gcc/5.4.0-x86_64/gcc/include/stdbool.h \
>  /build/gcc/5.4.0-x86_64/gcc/include/stddef.h
>
> This tells us that this uses not just compiler provided headers,
> but also ones provided by the platform (inttypes.h, limits.h,
> plus their descendants). I.e. we would not only become
> dependent upon whatever the platform library provides, but we'd
> also become dependent on there being a respective header
> installed in the first place. While that's quite likely a true
> assumption for native builds, I don't think we should assume this
> for cross builds.
>
> Additionally, looking through the resulting preprocessed file I also
> find
>
> typedef int __gwchar_t;
>
> typedef struct
>   {
>     long int quot;
>     long int rem;
>   } imaxdiv_t;
>
> extern intmax_t imaxabs (intmax_t __n) __attribute__ ((__nothrow__)) __attribute__ ((__const__));
>
> extern imaxdiv_t imaxdiv (intmax_t __numer, intmax_t __denom)
>       __attribute__ ((__nothrow__)) __attribute__ ((__const__));
>
> extern intmax_t strtoimax (__const char *__restrict __nptr,
>       char **__restrict __endptr, int __base) __attribute__ ((__nothrow__));
>
> extern uintmax_t strtoumax (__const char *__restrict __nptr,
>        char ** __restrict __endptr, int __base) __attribute__ ((__nothrow__));
>
> extern intmax_t wcstoimax (__const __gwchar_t *__restrict __nptr,
>       __gwchar_t **__restrict __endptr, int __base)
>      __attribute__ ((__nothrow__));
>
> extern uintmax_t wcstoumax (__const __gwchar_t *__restrict __nptr,
>        __gwchar_t ** __restrict __endptr, int __base)
>      __attribute__ ((__nothrow__));
>
> typedef struct {
>   long long __max_align_ll __attribute__((__aligned__(__alignof__(long long))));
>   long double __max_align_ld __attribute__((__aligned__(__alignof__(long double))));
> } max_align_t;
>
> none of which we want, I think (and I didn't even try to look at the
> set of resulting #define-s). Yes, the function declarations are benign
> as using them will result in linking failures, but it's still stuff getting
> added to the name space which we don't need or want.

Is this perhaps a side effect of using -std=gnu99?

Does it change when using -std=c99?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 6/6] xen/build: Use the system stdbool.h header

[Jan Beulich]

>>> On 22.06.16 at 13:24, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -480,7 +480,7 @@ void trace_exit_reason(u32 *irq_traced)
>   */
>  bool_t errata_c6_eoi_workaround(void)
>  {
> -    static bool_t fix_needed = -1;
> +    static int fix_needed = -1;

int8_t then please.

> --- a/xen/arch/x86/genapic/probe.c
> +++ b/xen/arch/x86/genapic/probe.c
> @@ -56,7 +56,8 @@ custom_param("apic", genapic_apic_force);
>  
>  void __init generic_apic_probe(void) 
>  { 
> -	int i, changed;
> +	bool_t changed;
> +	int i;

You mention it in the description, but I can't see what's wrong with
the current code (and hence I can't conclude what complaint the
compiler has to make).

Also now that you introduce plain bool - why do you use bool_t
here?

> @@ -46,4 +47,8 @@ typedef __u32 __be32;
>  typedef __u64 __le64;
>  typedef __u64 __be64;
>  
> +typedef _Bool bool_t;
> +#define test_and_set_bool(b)   xchg(&(b), 1)
> +#define test_and_clear_bool(b) xchg(&(b), 0)

I guess you then want to use true and false here (which we should
gradually switch to along with the bool_t -> bool transition)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xen/build: Use system headers

[Jan Beulich]

>>> On 22.06.16 at 14:33, <andrew.cooper3@citrix.com> wrote:
> On 22/06/16 13:26, Jan Beulich wrote:
>>>>> On 22.06.16 at 13:24, <andrew.cooper3@citrix.com> wrote:
>>> Make use of C standard freestanding headers, to avoid bugs in our own local
>>> versions of inttypes.h and booleans.
>> While the motivation to do this is certainly a good one, I see possible
>> problems resulting from this. These are best demonstrated by
>> compiling a C file containing just
>>
>> #include <inttypes.h>
>> #include <limits.h>
>> #include <stdarg.h>
>> #include <stdbool.h>
>> #include <stddef.h>
>> #include <stdint.h>
>>
>> with (among other relevant options) -MD, and with a cross compiler.
>> The resulting dependencies are
>>
>> std.o: std.c /usr/local/x86_64-unknown-linux/sys-include/inttypes.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/features.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/sys/cdefs.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/bits/wordsize.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/gnu/stubs.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/gnu/stubs-64.h \
>>  /build/gcc/5.4.0-x86_64/gcc/include/stdint.h \
>>  /build/gcc/5.4.0-x86_64/gcc/include/stdint-gcc.h \
>>  /build/gcc/5.4.0-x86_64/gcc/include-fixed/limits.h \
>>  /build/gcc/5.4.0-x86_64/gcc/include-fixed/syslimits.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/limits.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/bits/posix1_lim.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/bits/local_lim.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/linux/limits.h \
>>  /usr/local/x86_64-unknown-linux/sys-include/bits/posix2_lim.h \
>>  /build/gcc/5.4.0-x86_64/gcc/include/stdarg.h \
>>  /build/gcc/5.4.0-x86_64/gcc/include/stdbool.h \
>>  /build/gcc/5.4.0-x86_64/gcc/include/stddef.h
>>
>> This tells us that this uses not just compiler provided headers,
>> but also ones provided by the platform (inttypes.h, limits.h,
>> plus their descendants). I.e. we would not only become
>> dependent upon whatever the platform library provides, but we'd
>> also become dependent on there being a respective header
>> installed in the first place. While that's quite likely a true
>> assumption for native builds, I don't think we should assume this
>> for cross builds.
>>
>> Additionally, looking through the resulting preprocessed file I also
>> find
>>
>> typedef int __gwchar_t;
>>
>> typedef struct
>>   {
>>     long int quot;
>>     long int rem;
>>   } imaxdiv_t;
>>
>> extern intmax_t imaxabs (intmax_t __n) __attribute__ ((__nothrow__)) 
> __attribute__ ((__const__));
>>
>> extern imaxdiv_t imaxdiv (intmax_t __numer, intmax_t __denom)
>>       __attribute__ ((__nothrow__)) __attribute__ ((__const__));
>>
>> extern intmax_t strtoimax (__const char *__restrict __nptr,
>>       char **__restrict __endptr, int __base) __attribute__ ((__nothrow__));
>>
>> extern uintmax_t strtoumax (__const char *__restrict __nptr,
>>        char ** __restrict __endptr, int __base) __attribute__ 
> ((__nothrow__));
>>
>> extern intmax_t wcstoimax (__const __gwchar_t *__restrict __nptr,
>>       __gwchar_t **__restrict __endptr, int __base)
>>      __attribute__ ((__nothrow__));
>>
>> extern uintmax_t wcstoumax (__const __gwchar_t *__restrict __nptr,
>>        __gwchar_t ** __restrict __endptr, int __base)
>>      __attribute__ ((__nothrow__));
>>
>> typedef struct {
>>   long long __max_align_ll __attribute__((__aligned__(__alignof__(long 
> long))));
>>   long double __max_align_ld __attribute__((__aligned__(__alignof__(long 
> double))));
>> } max_align_t;
>>
>> none of which we want, I think (and I didn't even try to look at the
>> set of resulting #define-s). Yes, the function declarations are benign
>> as using them will result in linking failures, but it's still stuff getting
>> added to the name space which we don't need or want.
> 
> Is this perhaps a side effect of using -std=gnu99?
> 
> Does it change when using -std=c99?

Only the max_align_t one goes away (also when explicitly using
-std=gnu99). And the set of descendants of limits.h shrinks, but not
to an empty list.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 6/6] xen/build: Use the system stdbool.h header

[Andrew Cooper]

On 22/06/16 13:43, Jan Beulich wrote:
>>>> On 22.06.16 at 13:24, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/acpi/cpu_idle.c
>> +++ b/xen/arch/x86/acpi/cpu_idle.c
>> @@ -480,7 +480,7 @@ void trace_exit_reason(u32 *irq_traced)
>>   */
>>  bool_t errata_c6_eoi_workaround(void)
>>  {
>> -    static bool_t fix_needed = -1;
>> +    static int fix_needed = -1;
> int8_t then please.
>
>> --- a/xen/arch/x86/genapic/probe.c
>> +++ b/xen/arch/x86/genapic/probe.c
>> @@ -56,7 +56,8 @@ custom_param("apic", genapic_apic_force);
>>  
>>  void __init generic_apic_probe(void) 
>>  { 
>> -	int i, changed;
>> +	bool_t changed;
>> +	int i;
> You mention it in the description, but I can't see what's wrong with
> the current code (and hence I can't conclude what complaint the
> compiler has to make).

This one was indeed odd.

probe.c:64:2: error: suggest parentheses around assignment used as truth
value [-Werror=parentheses]
  cmdline_apic = changed = (genapic != NULL);
  ^

This is with

andrewcoop@andrewcoop:/local/xen.git/xen$ gcc --version
gcc (Debian 4.9.2-10) 4.9.2
...

Modifying the line to

cmdline_apic = (changed = (genapic != NULL));

also resolves the warning, so I suspect it is complaining about the
implicit truncation of bool = int = rvalue, without taking into account
that the rvalue is strictly 0 or 1.


>
> Also now that you introduce plain bool - why do you use bool_t
> here?

If we want to go in that direction, then certainly.  I was going to pose
the question bool_t/bool question later, but it would be my preference
to make a transition towards bool.

>
>> @@ -46,4 +47,8 @@ typedef __u32 __be32;
>>  typedef __u64 __le64;
>>  typedef __u64 __be64;
>>  
>> +typedef _Bool bool_t;
>> +#define test_and_set_bool(b)   xchg(&(b), 1)
>> +#define test_and_clear_bool(b) xchg(&(b), 0)
> I guess you then want to use true and false here (which we should
> gradually switch to along with the bool_t -> bool transition)?

I was hoping to find a better place for those to live, as types.h isn't
appropriate.  Still, the global types.h is better than having identical
copies in arch specific types.h

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Tim Deegan]

At 12:24 +0100 on 22 Jun (1466598248), Andrew Cooper wrote:
> The C standard defines two types of conforming implementation; hosted and
> freestanding.  A subset of the standard headers are the freestanding headers,
> requiring no library support at all to use, and therefore usable by Xen.
> 
> Unfortunately, -nostdinc is an overly large switch, and there is no
> alternative to only permit inclusion of the freestanding headers.  Removing it
> is unfortunate, as we lose the protection it offers, but anyone who does try
> to use other parts of the standard library will still fail to link.

I'm afraid I don't think this is a good idea:
 - Leaving the standard include path around in the Xen build means
   that the build may differ based on what (unrelated) libraries are
   installed on the build machines.
 - There are plenty of ways for an unexpected header to break things
   that don't fail at link time, e.g. macros and inlines.
 - "Freestanding" headers can bring in quite a lot of unrelated cruft.
   See Jan's email about linux/glibc, and I remember seeing similar
   things on solaris and *BSD when I tidied up stdarg.h. E.g. looking
   at two machines I'm working on today, on one of them,
   #include <limits.h> defines __packed, and on the other it does not.

Since what we have already works fine for all the compilers we
support, I think it ain't broke and we shouldn't fix it.

Nacked-by: Tim Deegan <tim@xen.org>

OTOH, I am in favour of s/bool_t/bool/g, at least wherever field size
is not an issue, and I can see an argument for moving the type and
limits definitions into files called stdint.h and limits.h.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Andrew Cooper]

On 22/06/16 14:12, Tim Deegan wrote:
> At 12:24 +0100 on 22 Jun (1466598248), Andrew Cooper wrote:
>> The C standard defines two types of conforming implementation; hosted and
>> freestanding.  A subset of the standard headers are the freestanding headers,
>> requiring no library support at all to use, and therefore usable by Xen.
>>
>> Unfortunately, -nostdinc is an overly large switch, and there is no
>> alternative to only permit inclusion of the freestanding headers.  Removing it
>> is unfortunate, as we lose the protection it offers, but anyone who does try
>> to use other parts of the standard library will still fail to link.
> I'm afraid I don't think this is a good idea:
>  - Leaving the standard include path around in the Xen build means
>    that the build may differ based on what (unrelated) libraries are
>    installed on the build machines.
>  - There are plenty of ways for an unexpected header to break things
>    that don't fail at link time, e.g. macros and inlines.
>  - "Freestanding" headers can bring in quite a lot of unrelated cruft.
>    See Jan's email about linux/glibc, and I remember seeing similar
>    things on solaris and *BSD when I tidied up stdarg.h. E.g. looking
>    at two machines I'm working on today, on one of them,
>    #include <limits.h> defines __packed, and on the other it does not.
>
> Since what we have already works fine for all the compilers we
> support, I think it ain't broke and we shouldn't fix it.

Except it is broken.

We cannot expect to use -Wformat and not the compiler provided
inttypes.h.  The sizes of constructs like "long long" are implementation
defined, not spec defined.  The compiler is perfectly free to choose
something which doesn't match our inttypes.h, and we would be in the
wrong when it fails to compile.

~Andrew

>
> Nacked-by: Tim Deegan <tim@xen.org>
>
> OTOH, I am in favour of s/bool_t/bool/g, at least wherever field size
> is not an issue, and I can see an argument for moving the type and
> limits definitions into files called stdint.h and limits.h.
>
> Cheers,
>
> Tim.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Tim Deegan]

At 14:46 +0100 on 13 Jul (1468421211), Andrew Cooper wrote:
> On 22/06/16 14:12, Tim Deegan wrote:
> > At 12:24 +0100 on 22 Jun (1466598248), Andrew Cooper wrote:
> >> The C standard defines two types of conforming implementation; hosted and
> >> freestanding.  A subset of the standard headers are the freestanding headers,
> >> requiring no library support at all to use, and therefore usable by Xen.
> >>
> >> Unfortunately, -nostdinc is an overly large switch, and there is no
> >> alternative to only permit inclusion of the freestanding headers.  Removing it
> >> is unfortunate, as we lose the protection it offers, but anyone who does try
> >> to use other parts of the standard library will still fail to link.
> > I'm afraid I don't think this is a good idea:
> >  - Leaving the standard include path around in the Xen build means
> >    that the build may differ based on what (unrelated) libraries are
> >    installed on the build machines.
> >  - There are plenty of ways for an unexpected header to break things
> >    that don't fail at link time, e.g. macros and inlines.
> >  - "Freestanding" headers can bring in quite a lot of unrelated cruft.
> >    See Jan's email about linux/glibc, and I remember seeing similar
> >    things on solaris and *BSD when I tidied up stdarg.h. E.g. looking
> >    at two machines I'm working on today, on one of them,
> >    #include <limits.h> defines __packed, and on the other it does not.
> >
> > Since what we have already works fine for all the compilers we
> > support, I think it ain't broke and we shouldn't fix it.
> 
> Except it is broken.

Broken in theory or actually causing a problem right now?

> We cannot expect to use -Wformat and not the compiler provided
> inttypes.h.

Since we're not using the compiler-provided printf(), I think that
we're on pretty thin ice with -Wformat anyway.  But as long as our
own PRI* macros match our type definitions, all should be well, right?

> The sizes of constructs like "long long" are implementation
> defined, not spec defined.  The compiler is perfectly free to choose
> something which doesn't match our inttypes.h, and we would be in the
> wrong when it fails to compile.

If a compiler we support does that, we can update our integer type
definitions, like we do for attribute annotations, &c.

BTW, I can absolutely see the argument for deferring to the compiler
in C implementation details.  I think that would have to include
removing all the implementation-reserved names from Xen so we don't
clash with compiler headers.

But in practice it seems to be messy enough to be better avoided --
having /usr/include on the search path is pretty silly.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Jan Beulich]

>>> On 13.07.16 at 17:17, <tim@xen.org> wrote:
> At 14:46 +0100 on 13 Jul (1468421211), Andrew Cooper wrote:
>> We cannot expect to use -Wformat and not the compiler provided
>> inttypes.h.
> 
> Since we're not using the compiler-provided printf(), I think that
> we're on pretty thin ice with -Wformat anyway.

Why? As long as we don't assign non-standard meaning to standard
format specifiers, I think we ought to be fine. After all libc's sprintf()
isn't compiler provided either (it's not even in the same package, not
to speak of there being various alternatives to glibc).

>  But as long as our
> own PRI* macros match our type definitions, all should be well, right?

I think so.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Jan Beulich]

>>> On 13.07.16 at 15:46, <andrew.cooper3@citrix.com> wrote:
> On 22/06/16 14:12, Tim Deegan wrote:
>> At 12:24 +0100 on 22 Jun (1466598248), Andrew Cooper wrote:
>>> The C standard defines two types of conforming implementation; hosted and
>>> freestanding.  A subset of the standard headers are the freestanding 
> headers,
>>> requiring no library support at all to use, and therefore usable by Xen.
>>>
>>> Unfortunately, -nostdinc is an overly large switch, and there is no
>>> alternative to only permit inclusion of the freestanding headers.  Removing 
> it
>>> is unfortunate, as we lose the protection it offers, but anyone who does try
>>> to use other parts of the standard library will still fail to link.
>> I'm afraid I don't think this is a good idea:
>>  - Leaving the standard include path around in the Xen build means
>>    that the build may differ based on what (unrelated) libraries are
>>    installed on the build machines.
>>  - There are plenty of ways for an unexpected header to break things
>>    that don't fail at link time, e.g. macros and inlines.
>>  - "Freestanding" headers can bring in quite a lot of unrelated cruft.
>>    See Jan's email about linux/glibc, and I remember seeing similar
>>    things on solaris and *BSD when I tidied up stdarg.h. E.g. looking
>>    at two machines I'm working on today, on one of them,
>>    #include <limits.h> defines __packed, and on the other it does not.
>>
>> Since what we have already works fine for all the compilers we
>> support, I think it ain't broke and we shouldn't fix it.
> 
> Except it is broken.
> 
> We cannot expect to use -Wformat and not the compiler provided
> inttypes.h.  The sizes of constructs like "long long" are implementation
> defined, not spec defined.  The compiler is perfectly free to choose
> something which doesn't match our inttypes.h, and we would be in the
> wrong when it fails to compile.

Well, it's clearly sub-optimal to use types like "long" or "long long" to
set up our fixed width ones; we would really better make use of
__attribute__((__mode__(...))) here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel