All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] xen/types: Alter typedef for bool_t
@ 2016-08-01 13:14 Andrew Cooper
  2016-08-01 13:14 ` [PATCH 2/3] xen/types: Correct the definition of uintptr_t Andrew Cooper
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Andrew Cooper @ 2016-08-01 13:14 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Stefano Stabellini, Tim Deegan, Jan Beulich

As xen/stdbool.h is included, the typedef should use bool rather than _Bool.

Suggested-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/include/xen/types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index 78410de..c8092d0 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -61,7 +61,7 @@ typedef __u64 __be64;
 
 typedef unsigned long uintptr_t;
 
-typedef _Bool bool_t;
+typedef bool bool_t;
 #define test_and_set_bool(b)   xchg(&(b), true)
 #define test_and_clear_bool(b) xchg(&(b), false)
 
-- 
2.1.4


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

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

* [PATCH 2/3] xen/types: Correct the definition of uintptr_t
  2016-08-01 13:14 [PATCH 1/3] xen/types: Alter typedef for bool_t Andrew Cooper
@ 2016-08-01 13:14 ` Andrew Cooper
  2016-08-01 15:51   ` Jan Beulich
  2016-08-01 13:14 ` [PATCH 3/3] xen/common: Sort the obj build order Andrew Cooper
  2016-08-01 15:44 ` [PATCH 1/3] xen/types: Alter typedef for bool_t Jan Beulich
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2016-08-01 13:14 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Tim Deegan, Jan Beulich

uintptr_t is specified as unsigned int in 32bit, not unsigned long.  This is
why, when copying inttypes.h from GCC, the use of PRIxPTR and similar is
broken for 32bit builds.

Fix the identified breakage with ELF_PRPTRVAL

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
 xen/include/xen/libelf.h | 10 +---------
 xen/include/xen/types.h  |  4 ++++
 2 files changed, 5 insertions(+), 9 deletions(-)

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 c8092d0..44ae81c 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -59,7 +59,11 @@ typedef __u32 __be32;
 typedef __u64 __le64;
 typedef __u64 __be64;
 
+#if BITS_PER_LONG == 64
 typedef unsigned long uintptr_t;
+#elif BITS_PER_LONG == 32
+typedef unsigned int uintptr_t;
+#endif
 
 typedef bool bool_t;
 #define test_and_set_bool(b)   xchg(&(b), true)
-- 
2.1.4


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

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

* [PATCH 3/3] xen/common: Sort the obj build order
  2016-08-01 13:14 [PATCH 1/3] xen/types: Alter typedef for bool_t Andrew Cooper
  2016-08-01 13:14 ` [PATCH 2/3] xen/types: Correct the definition of uintptr_t Andrew Cooper
@ 2016-08-01 13:14 ` Andrew Cooper
  2016-08-01 15:52   ` Jan Beulich
  2016-08-01 15:44 ` [PATCH 1/3] xen/types: Alter typedef for bool_t Jan Beulich
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2016-08-01 13:14 UTC (permalink / raw)
  To: Xen-devel
  Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Tim Deegan,
	Jan Beulich

Suggested-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@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>
---
 xen/common/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/Makefile b/xen/common/Makefile
index f8123c2..c2e6846 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,4 +1,5 @@
 obj-y += bitmap.o
+obj-y += bsearch.o
 obj-$(CONFIG_CORE_PARKING) += core_parking.o
 obj-y += cpu.o
 obj-y += cpupool.o
@@ -43,7 +44,6 @@ obj-y += schedule.o
 obj-y += shutdown.o
 obj-y += softirq.o
 obj-y += sort.o
-obj-y += bsearch.o
 obj-y += smp.o
 obj-y += spinlock.o
 obj-y += stop_machine.o
-- 
2.1.4


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

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

* Re: [PATCH 1/3] xen/types: Alter typedef for bool_t
  2016-08-01 13:14 [PATCH 1/3] xen/types: Alter typedef for bool_t Andrew Cooper
  2016-08-01 13:14 ` [PATCH 2/3] xen/types: Correct the definition of uintptr_t Andrew Cooper
  2016-08-01 13:14 ` [PATCH 3/3] xen/common: Sort the obj build order Andrew Cooper
@ 2016-08-01 15:44 ` Jan Beulich
  2 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2016-08-01 15:44 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Stefano Stabellini, Tim Deegan, Xen-devel

>>> On 01.08.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
> As xen/stdbool.h is included, the typedef should use bool rather than _Bool.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>

I think this implies it, but just in case you think it doesn't ...

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

...:
Acked-by: Jan Beulich <jbeulich@suse.com>


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

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

* Re: [PATCH 2/3] xen/types: Correct the definition of uintptr_t
  2016-08-01 13:14 ` [PATCH 2/3] xen/types: Correct the definition of uintptr_t Andrew Cooper
@ 2016-08-01 15:51   ` Jan Beulich
  2016-08-01 16:08     ` Andrew Cooper
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2016-08-01 15:51 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Tim Deegan, Xen-devel

>>> On 01.08.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
> uintptr_t is specified as unsigned int in 32bit, not unsigned long.  This is
> why, when copying inttypes.h from GCC, the use of PRIxPTR and similar is
> broken for 32bit builds.

I don't think this is strictly the case, i.e. doing it this way still ties
us to internal workings of the compiler (as there is room for targets
to customize base types used for derived ones). Could you try
whether ...

> --- a/xen/include/xen/types.h
> +++ b/xen/include/xen/types.h
> @@ -59,7 +59,11 @@ typedef __u32 __be32;
>  typedef __u64 __le64;
>  typedef __u64 __be64;
>  
> +#if BITS_PER_LONG == 64
>  typedef unsigned long uintptr_t;
> +#elif BITS_PER_LONG == 32
> +typedef unsigned int uintptr_t;
> +#endif

...

typedef unsigned int __attribute__((__mode__(__pointer__))) uintptr_t;

works both ways (32- and 64-bit)?

Jan


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

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

* Re: [PATCH 3/3] xen/common: Sort the obj build order
  2016-08-01 13:14 ` [PATCH 3/3] xen/common: Sort the obj build order Andrew Cooper
@ 2016-08-01 15:52   ` Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2016-08-01 15:52 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: George Dunlap, Stefano Stabellini, Tim Deegan, Xen-devel

>>> On 01.08.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
> Suggested-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Just like before, in case it's not implied:
Acked-by: Jan Beulich <jbeulich@suse.com>


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

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

* Re: [PATCH 2/3] xen/types: Correct the definition of uintptr_t
  2016-08-01 15:51   ` Jan Beulich
@ 2016-08-01 16:08     ` Andrew Cooper
  2016-08-01 16:26       ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2016-08-01 16:08 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Julien Grall, Stefano Stabellini, Tim Deegan, Xen-devel

On 01/08/16 16:51, Jan Beulich wrote:
>>>> On 01.08.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
>> uintptr_t is specified as unsigned int in 32bit, not unsigned long.  This is
>> why, when copying inttypes.h from GCC, the use of PRIxPTR and similar is
>> broken for 32bit builds.
> I don't think this is strictly the case, i.e. doing it this way still ties
> us to internal workings of the compiler (as there is room for targets
> to customize base types used for derived ones). Could you try
> whether ...

This is the entire point I am trying to make that what we are doing is
currently unsafe if we are not using the header files to match the
compiler in use.

>
>> --- a/xen/include/xen/types.h
>> +++ b/xen/include/xen/types.h
>> @@ -59,7 +59,11 @@ typedef __u32 __be32;
>>  typedef __u64 __le64;
>>  typedef __u64 __be64;
>>  
>> +#if BITS_PER_LONG == 64
>>  typedef unsigned long uintptr_t;
>> +#elif BITS_PER_LONG == 32
>> +typedef unsigned int uintptr_t;
>> +#endif
> ...
>
> typedef unsigned int __attribute__((__mode__(__pointer__))) uintptr_t;
>
> works both ways (32- and 64-bit)?

It does appear to work.

~Andrew

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

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

* Re: [PATCH 2/3] xen/types: Correct the definition of uintptr_t
  2016-08-01 16:08     ` Andrew Cooper
@ 2016-08-01 16:26       ` Jan Beulich
  2016-08-01 16:34         ` [PATCH v2 " Andrew Cooper
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2016-08-01 16:26 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Tim Deegan, Xen-devel

>>> On 01.08.16 at 18:08, <andrew.cooper3@citrix.com> wrote:
> On 01/08/16 16:51, Jan Beulich wrote:
>>>>> On 01.08.16 at 15:14, <andrew.cooper3@citrix.com> wrote:
>>> uintptr_t is specified as unsigned int in 32bit, not unsigned long.  This is
>>> why, when copying inttypes.h from GCC, the use of PRIxPTR and similar is
>>> broken for 32bit builds.
>> I don't think this is strictly the case, i.e. doing it this way still ties
>> us to internal workings of the compiler (as there is room for targets
>> to customize base types used for derived ones). Could you try
>> whether ...
> 
> This is the entire point I am trying to make that what we are doing is
> currently unsafe if we are not using the header files to match the
> compiler in use.

That's really only a problem if we don't use what the compiler
provides us to make up those header clones of ours, like e.g. (as
said) the mode attribute.

Jan


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

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

* [PATCH v2 2/3] xen/types: Correct the definition of uintptr_t
  2016-08-01 16:26       ` Jan Beulich
@ 2016-08-01 16:34         ` Andrew Cooper
  2016-08-02  7:19           ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2016-08-01 16:34 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Tim Deegan, Jan Beulich

uintptr_t is specified as unsigned int in 32bit, not unsigned long.  This is
why, when copying inttypes.h from GCC, the use of PRIxPTR and similar is
broken for 32bit builds.

Use __attribute__((__mode__(__pointer__))) to get the compilers default
pointer type, which matches the pre-existing inttypes.h

Fix the identified breakage with ELF_PRPTRVAL

Compile tested on all architectures, with a manual printk() to trigger any
potential -Wformat issues.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>

v2:
 * Use mode rather than extra typedefs
---
 xen/include/xen/libelf.h | 10 +---------
 xen/include/xen/types.h  |  2 +-
 2 files changed, 2 insertions(+), 10 deletions(-)

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 c8092d0..7bdc83b 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -59,7 +59,7 @@ typedef __u32 __be32;
 typedef __u64 __le64;
 typedef __u64 __be64;
 
-typedef unsigned long uintptr_t;
+typedef unsigned int __attribute__((__mode__(__pointer__))) uintptr_t;
 
 typedef bool bool_t;
 #define test_and_set_bool(b)   xchg(&(b), true)
-- 
2.1.4


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

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

* Re: [PATCH v2 2/3] xen/types: Correct the definition of uintptr_t
  2016-08-01 16:34         ` [PATCH v2 " Andrew Cooper
@ 2016-08-02  7:19           ` Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2016-08-02  7:19 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Tim Deegan, Xen-devel

>>> On 01.08.16 at 18:34, <andrew.cooper3@citrix.com> wrote:
> uintptr_t is specified as unsigned int in 32bit, not unsigned long.  This is
> why, when copying inttypes.h from GCC, the use of PRIxPTR and similar is
> broken for 32bit builds.
> 
> Use __attribute__((__mode__(__pointer__))) to get the compilers default
> pointer type, which matches the pre-existing inttypes.h
> 
> Fix the identified breakage with ELF_PRPTRVAL
> 
> Compile tested on all architectures, with a manual printk() to trigger any
> potential -Wformat issues.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>


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

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

end of thread, other threads:[~2016-08-02  7:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-01 13:14 [PATCH 1/3] xen/types: Alter typedef for bool_t Andrew Cooper
2016-08-01 13:14 ` [PATCH 2/3] xen/types: Correct the definition of uintptr_t Andrew Cooper
2016-08-01 15:51   ` Jan Beulich
2016-08-01 16:08     ` Andrew Cooper
2016-08-01 16:26       ` Jan Beulich
2016-08-01 16:34         ` [PATCH v2 " Andrew Cooper
2016-08-02  7:19           ` Jan Beulich
2016-08-01 13:14 ` [PATCH 3/3] xen/common: Sort the obj build order Andrew Cooper
2016-08-01 15:52   ` Jan Beulich
2016-08-01 15:44 ` [PATCH 1/3] xen/types: Alter typedef for bool_t Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.