[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/2] xen/build: Use C99 booleans



Hi Andrew,

On 14/07/16 16:58, Andrew Cooper wrote:
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 (which triggers a [-Werror=parentheses] warning on Debian
Jessie).

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.

I will add an item in my todo list to clean-up the ARM code. Is there any plan to retire either bool_t or bool?

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

From ARM bits:

Acked-by: Julien Grall <julien.grall@xxxxxxx>

---
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>

v2:
  * Leave the matter of std libaries to one side for now.  Fixing bool_t is far
    more important, and can be done without making the std include issue any
    worse.
  * Type tweaks, per v1 review.
---
  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/types.h              | 6 ++++++
  9 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 976f97b..a4bc55a 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 3326122..a2eb1da 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..7e235a3 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 int8_t 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..860201e 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 changed;
+       int i;

        record_boot_APIC_mode();

diff --git a/xen/include/asm-arm/types.h b/xen/include/asm-arm/types.h
index 09e5455..71d2e42 100644
--- a/xen/include/asm-arm/types.h
+++ b/xen/include/asm-arm/types.h
@@ -62,10 +62,6 @@ typedef unsigned long size_t;
  #endif
  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 b82fa58..e75b744 100644
--- a/xen/include/asm-x86/types.h
+++ b/xen/include/asm-x86/types.h
@@ -41,10 +41,6 @@ typedef unsigned long size_t;
  #endif
  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/types.h b/xen/include/xen/types.h
index 8596ded..78410de 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -1,6 +1,8 @@
  #ifndef __TYPES_H__
  #define __TYPES_H__

+#include <xen/stdbool.h>
+
  #include <asm/types.h>

  #define BITS_TO_LONGS(bits) \
@@ -59,4 +61,8 @@ typedef __u64 __be64;

  typedef unsigned long uintptr_t;

+typedef _Bool bool_t;
+#define test_and_set_bool(b)   xchg(&(b), true)
+#define test_and_clear_bool(b) xchg(&(b), false)
+
  #endif /* __TYPES_H__ */


--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.