Instead of having multiple files defining offsetof(), add the
definition to tools/include/xen-tools/common-macros.h.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- don't modify tools/firmware/include/stddef.h (Jan Beulich)
---
 tools/firmware/hvmloader/util.h         | 3 ---
 tools/include/xen-tools/common-macros.h | 4 ++++
 tools/libfsimage/Rules.mk               | 2 ++
 tools/libfsimage/xfs/fsys_xfs.c         | 4 +---
 tools/libs/vchan/init.c                 | 4 ----
 tools/tests/vhpet/emul.h                | 2 --
 6 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index e04990ee97..7249773eeb 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -30,9 +30,6 @@ enum {
 #define SEL_DATA32          0x0020
 #define SEL_CODE64          0x0028
 
-#undef offsetof
-#define offsetof(t, m) ((unsigned long)&((t *)0)->m)
-
 #undef NULL
 #define NULL ((void*)0)
 
diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
index b046ab48bf..b149be8365 100644
--- a/tools/include/xen-tools/common-macros.h
+++ b/tools/include/xen-tools/common-macros.h
@@ -80,4 +80,8 @@
     typeof( ((type *)0)->member ) *__mptr = (ptr);        \
     (type *)( (char *)__mptr - offsetof(type,member) );})
 
+#ifndef offsetof
+#define offsetof(a, b) __builtin_offsetof(a, b)
+#endif
+
 #endif	/* __XEN_TOOLS_COMMON_MACROS__ */
diff --git a/tools/libfsimage/Rules.mk b/tools/libfsimage/Rules.mk
index cf37d6cb0d..85674f2379 100644
--- a/tools/libfsimage/Rules.mk
+++ b/tools/libfsimage/Rules.mk
@@ -3,6 +3,8 @@ include $(XEN_ROOT)/tools/libfsimage/common.mk
 FSLIB = fsimage.so
 TARGETS += $(FSLIB)
 
+CFLAGS += $(CFLAGS_xeninclude)
+
 .PHONY: all
 all: $(TARGETS)
 
diff --git a/tools/libfsimage/xfs/fsys_xfs.c b/tools/libfsimage/xfs/fsys_xfs.c
index d735a88e55..f562daaef0 100644
--- a/tools/libfsimage/xfs/fsys_xfs.c
+++ b/tools/libfsimage/xfs/fsys_xfs.c
@@ -19,6 +19,7 @@
 
 #include <xenfsimage_grub.h>
 #include "xfs.h"
+#include <xen-tools/common-macros.h>
 
 #define MAX_LINK_COUNT	8
 
@@ -182,9 +183,6 @@ fsb2daddr (xfs_fsblock_t fsbno)
 			 (xfs_agblock_t)(fsbno & mask32lo(xfs.agblklog)));
 }
 
-#undef offsetof
-#define offsetof(t,m)	((size_t)&(((t *)0)->m))
-
 static inline int
 btroot_maxrecs (fsi_file_t *ffi)
 {
diff --git a/tools/libs/vchan/init.c b/tools/libs/vchan/init.c
index 021e1f29e1..ad4cc66d2c 100644
--- a/tools/libs/vchan/init.c
+++ b/tools/libs/vchan/init.c
@@ -69,10 +69,6 @@
 #define MAX_RING_SHIFT 20
 #define MAX_RING_SIZE (1 << MAX_RING_SHIFT)
 
-#ifndef offsetof
-#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
-#endif
-
 static int init_gnt_srv(struct libxenvchan *ctrl, int domain)
 {
 	int pages_left = ctrl->read.order >= PAGE_SHIFT ? 1 << (ctrl->read.order - PAGE_SHIFT) : 0;
diff --git a/tools/tests/vhpet/emul.h b/tools/tests/vhpet/emul.h
index 6af880cd43..7b3ee88fb5 100644
--- a/tools/tests/vhpet/emul.h
+++ b/tools/tests/vhpet/emul.h
@@ -115,8 +115,6 @@ enum
     NR_COMMON_SOFTIRQS
 };
 
-#define offsetof(t, m) ((unsigned long )&((t *)0)->m)
-
 struct domain;
 
 struct vcpu
-- 
2.35.3

On 06.03.2023 08:21, Juergen Gross wrote:
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -80,4 +80,8 @@
>      typeof( ((type *)0)->member ) *__mptr = (ptr);        \
>      (type *)( (char *)__mptr - offsetof(type,member) );})
>  
> +#ifndef offsetof
> +#define offsetof(a, b) __builtin_offsetof(a, b)
> +#endif

How's this going to work reliably with parties perhaps also including
stddef.h, which also is expected to define this macro, and which may
do so with a different sequence of tokens (even if that ends up
having the same effect)? There shouldn't be a need to define this
macro for any "normal" environments, e.g. ...

> --- a/tools/libfsimage/xfs/fsys_xfs.c
> +++ b/tools/libfsimage/xfs/fsys_xfs.c
> @@ -19,6 +19,7 @@
>  
>  #include <xenfsimage_grub.h>
>  #include "xfs.h"
> +#include <xen-tools/common-macros.h>
>  
>  #define MAX_LINK_COUNT	8
>  
> @@ -182,9 +183,6 @@ fsb2daddr (xfs_fsblock_t fsbno)
>  			 (xfs_agblock_t)(fsbno & mask32lo(xfs.agblklog)));
>  }
>  
> -#undef offsetof
> -#define offsetof(t,m)	((size_t)&(((t *)0)->m))
> -
>  static inline int
>  btroot_maxrecs (fsi_file_t *ffi)
>  {

... this library or ...

> --- a/tools/libs/vchan/init.c
> +++ b/tools/libs/vchan/init.c
> @@ -69,10 +69,6 @@
>  #define MAX_RING_SHIFT 20
>  #define MAX_RING_SIZE (1 << MAX_RING_SHIFT)
>  
> -#ifndef offsetof
> -#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
> -#endif
> -
>  static int init_gnt_srv(struct libxenvchan *ctrl, int domain)
>  {
>  	int pages_left = ctrl->read.order >= PAGE_SHIFT ? 1 << (ctrl->read.order - PAGE_SHIFT) : 0;

... this tool or ...

> --- a/tools/tests/vhpet/emul.h
> +++ b/tools/tests/vhpet/emul.h
> @@ -115,8 +115,6 @@ enum
>      NR_COMMON_SOFTIRQS
>  };
>  
> -#define offsetof(t, m) ((unsigned long )&((t *)0)->m)
> -
>  struct domain;
>  
>  struct vcpu

... this test binary (which is an ordinary host one as well). They all
should include stddef.h and be done.

Jan

On 06.03.23 09:00, Jan Beulich wrote:
> On 06.03.2023 08:21, Juergen Gross wrote:
>> --- a/tools/include/xen-tools/common-macros.h
>> +++ b/tools/include/xen-tools/common-macros.h
>> @@ -80,4 +80,8 @@
>>       typeof( ((type *)0)->member ) *__mptr = (ptr);        \
>>       (type *)( (char *)__mptr - offsetof(type,member) );})
>>   
>> +#ifndef offsetof
>> +#define offsetof(a, b) __builtin_offsetof(a, b)
>> +#endif
> 
> How's this going to work reliably with parties perhaps also including
> stddef.h, which also is expected to define this macro, and which may
> do so with a different sequence of tokens (even if that ends up
> having the same effect)? There shouldn't be a need to define this
> macro for any "normal" environments, e.g. ...
> 
>> --- a/tools/libfsimage/xfs/fsys_xfs.c
>> +++ b/tools/libfsimage/xfs/fsys_xfs.c
>> @@ -19,6 +19,7 @@
>>   
>>   #include <xenfsimage_grub.h>
>>   #include "xfs.h"
>> +#include <xen-tools/common-macros.h>
>>   
>>   #define MAX_LINK_COUNT	8
>>   
>> @@ -182,9 +183,6 @@ fsb2daddr (xfs_fsblock_t fsbno)
>>   			 (xfs_agblock_t)(fsbno & mask32lo(xfs.agblklog)));
>>   }
>>   
>> -#undef offsetof
>> -#define offsetof(t,m)	((size_t)&(((t *)0)->m))
>> -
>>   static inline int
>>   btroot_maxrecs (fsi_file_t *ffi)
>>   {
> 
> ... this library or ...
> 
>> --- a/tools/libs/vchan/init.c
>> +++ b/tools/libs/vchan/init.c
>> @@ -69,10 +69,6 @@
>>   #define MAX_RING_SHIFT 20
>>   #define MAX_RING_SIZE (1 << MAX_RING_SHIFT)
>>   
>> -#ifndef offsetof
>> -#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
>> -#endif
>> -
>>   static int init_gnt_srv(struct libxenvchan *ctrl, int domain)
>>   {
>>   	int pages_left = ctrl->read.order >= PAGE_SHIFT ? 1 << (ctrl->read.order - PAGE_SHIFT) : 0;
> 
> ... this tool or ...
> 
>> --- a/tools/tests/vhpet/emul.h
>> +++ b/tools/tests/vhpet/emul.h
>> @@ -115,8 +115,6 @@ enum
>>       NR_COMMON_SOFTIRQS
>>   };
>>   
>> -#define offsetof(t, m) ((unsigned long )&((t *)0)->m)
>> -
>>   struct domain;
>>   
>>   struct vcpu
> 
> ... this test binary (which is an ordinary host one as well). They all
> should include stddef.h and be done.

Yes, this is probably a better way to handle this.

So I think this patch should be dropped.

I'll send out another small series removing the private offsetof()
definitions and using stddef.h instead. This should include hvmloader,
which can use the tools/firmware/include/stddef.h variant.


Juergen