diff mbox series

[v4,1/5] tools: add container_of() macro to xen-tools/common-macros.h

Message ID 20230322120844.19657-2-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series tools: use xen-tools/libs.h for common definitions | expand

Commit Message

Jürgen Groß March 22, 2023, 12:08 p.m. UTC 
Instead of having 4 identical copies of the definition of a
container_of() macro in different tools header files, add that macro
to xen-tools/common-macros.h and use that instead.

Delete the other copies of that macro.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
There is a similar macro CONTAINER_OF() defined in
tools/include/xentoolcore_internal.h, which allows to not only use a
type for the 2nd parameter, but a variable, too.
I'd like to get rid of that macro as well, but there are lots of use
cases especially in libxl. Any thoughts regarding that macro?
I could either:
- don't touch it at all
- enhance container_of() like CONTAINER_OF() and replace all use cases
  of CONTAINER_OF() with container_of()
- replace the few CONTAINER_OF() users outside libxl with container_of()
  and define CONTAINER_OF() in e.g. libxl_internal.h
- replace all CONTAINER_OF() use cases with container_of(), including
  the change from (.., var, ..) to (.., type, ...).

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/include/xen-tools/common-macros.h | 4 ++++
 tools/tests/vpci/emul.h                 | 6 +-----
 tools/tests/x86_emulator/x86-emulate.h  | 5 -----
 tools/xenstore/list.h                   | 6 ++----
 4 files changed, 7 insertions(+), 14 deletions(-)

Comments

Jan Beulich March 22, 2023, 12:34 p.m. UTC | #1 
On 22.03.2023 13:08, Juergen Gross wrote:
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -76,4 +76,8 @@
>  #define __must_check __attribute__((__warn_unused_result__))
>  #endif
>  
> +#define container_of(ptr, type, member) ({                \
> +    typeof( ((type *)0)->member ) *__mptr = (ptr);        \
> +    (type *)( (char *)__mptr - offsetof(type,member) );})

Can the variant used here please be closer to ...

> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -56,11 +56,6 @@
>  
>  #define cf_check /* No Control Flow Integriy checking */
>  
> -#define container_of(ptr, type, member) ({             \
> -    typeof(((type *)0)->member) *mptr__ = (ptr);       \
> -    (type *)((char *)mptr__ - offsetof(type, member)); \
> -})

... this rather than ...

> --- a/tools/xenstore/list.h
> +++ b/tools/xenstore/list.h
> @@ -3,6 +3,8 @@
>  /* Taken from Linux kernel code, but de-kernelized for userspace. */
>  #include <stddef.h>
>  
> +#include <xen-tools/common-macros.h>
> +
>  #undef LIST_HEAD_INIT
>  #undef LIST_HEAD
>  #undef INIT_LIST_HEAD
> @@ -15,10 +17,6 @@
>  #define LIST_POISON1  ((void *) 0x00100100)
>  #define LIST_POISON2  ((void *) 0x00200200)
>  
> -#define container_of(ptr, type, member) ({			\
> -        typeof( ((type *)0)->member ) *__mptr = (ptr);	\
> -        (type *)( (char *)__mptr - offsetof(type,member) );})

... this, both formatting-wise (excess blanks) and local-variable-
naming-wise (trailing underscores instead of leading ones)? (If I was
the one to commit this, I'd be happy to make the adjustment at that
time.) Then
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan
Jürgen Groß March 22, 2023, 12:44 p.m. UTC | #2 
On 22.03.23 13:34, Jan Beulich wrote:
> On 22.03.2023 13:08, Juergen Gross wrote:
>> --- a/tools/include/xen-tools/common-macros.h
>> +++ b/tools/include/xen-tools/common-macros.h
>> @@ -76,4 +76,8 @@
>>   #define __must_check __attribute__((__warn_unused_result__))
>>   #endif
>>   
>> +#define container_of(ptr, type, member) ({                \
>> +    typeof( ((type *)0)->member ) *__mptr = (ptr);        \
>> +    (type *)( (char *)__mptr - offsetof(type,member) );})
> 
> Can the variant used here please be closer to ...
> 
>> --- a/tools/tests/x86_emulator/x86-emulate.h
>> +++ b/tools/tests/x86_emulator/x86-emulate.h
>> @@ -56,11 +56,6 @@
>>   
>>   #define cf_check /* No Control Flow Integriy checking */
>>   
>> -#define container_of(ptr, type, member) ({             \
>> -    typeof(((type *)0)->member) *mptr__ = (ptr);       \
>> -    (type *)((char *)mptr__ - offsetof(type, member)); \
>> -})
> 
> ... this rather than ...
> 
>> --- a/tools/xenstore/list.h
>> +++ b/tools/xenstore/list.h
>> @@ -3,6 +3,8 @@
>>   /* Taken from Linux kernel code, but de-kernelized for userspace. */
>>   #include <stddef.h>
>>   
>> +#include <xen-tools/common-macros.h>
>> +
>>   #undef LIST_HEAD_INIT
>>   #undef LIST_HEAD
>>   #undef INIT_LIST_HEAD
>> @@ -15,10 +17,6 @@
>>   #define LIST_POISON1  ((void *) 0x00100100)
>>   #define LIST_POISON2  ((void *) 0x00200200)
>>   
>> -#define container_of(ptr, type, member) ({			\
>> -        typeof( ((type *)0)->member ) *__mptr = (ptr);	\
>> -        (type *)( (char *)__mptr - offsetof(type,member) );})
> 
> ... this, both formatting-wise (excess blanks) and local-variable-
> naming-wise (trailing underscores instead of leading ones)? (If I was
> the one to commit this, I'd be happy to make the adjustment at that
> time.) Then

Yes, absolutely fine with me.

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

Thanks,


Juergen
diff mbox series

Patch

diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
index a372b9ecf2..b046ab48bf 100644
--- a/tools/include/xen-tools/common-macros.h
+++ b/tools/include/xen-tools/common-macros.h
@@ -76,4 +76,8 @@ 
 #define __must_check __attribute__((__warn_unused_result__))
 #endif
 
+#define container_of(ptr, type, member) ({                \
+    typeof( ((type *)0)->member ) *__mptr = (ptr);        \
+    (type *)( (char *)__mptr - offsetof(type,member) );})
+
 #endif	/* __XEN_TOOLS_COMMON_MACROS__ */
diff --git a/tools/tests/vpci/emul.h b/tools/tests/vpci/emul.h
index f03e3a56d1..7169a2ea02 100644
--- a/tools/tests/vpci/emul.h
+++ b/tools/tests/vpci/emul.h
@@ -27,11 +27,7 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 
-#define container_of(ptr, type, member) ({                      \
-        typeof(((type *)0)->member) *mptr = (ptr);              \
-                                                                \
-        (type *)((char *)mptr - offsetof(type, member));        \
-})
+#include <xen-tools/common-macros.h>
 
 #define smp_wmb()
 #define prefetch(x) __builtin_prefetch(x)
diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
index 46d4e43cea..1af986f78d 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -56,11 +56,6 @@ 
 
 #define cf_check /* No Control Flow Integriy checking */
 
-#define container_of(ptr, type, member) ({             \
-    typeof(((type *)0)->member) *mptr__ = (ptr);       \
-    (type *)((char *)mptr__ - offsetof(type, member)); \
-})
-
 #define AC_(n,t) (n##t)
 #define _AC(n,t) AC_(n,t)
 
diff --git a/tools/xenstore/list.h b/tools/xenstore/list.h
index b17d13e0ec..a464a38b61 100644
--- a/tools/xenstore/list.h
+++ b/tools/xenstore/list.h
@@ -3,6 +3,8 @@ 
 /* Taken from Linux kernel code, but de-kernelized for userspace. */
 #include <stddef.h>
 
+#include <xen-tools/common-macros.h>
+
 #undef LIST_HEAD_INIT
 #undef LIST_HEAD
 #undef INIT_LIST_HEAD
@@ -15,10 +17,6 @@ 
 #define LIST_POISON1  ((void *) 0x00100100)
 #define LIST_POISON2  ((void *) 0x00200200)
 
-#define container_of(ptr, type, member) ({			\
-        typeof( ((type *)0)->member ) *__mptr = (ptr);	\
-        (type *)( (char *)__mptr - offsetof(type,member) );})
-
 /*
  * Simple doubly linked list implementation.
  *