diff mbox series

[v2,4/4] tools: add offsetof() to xen-tools/common-macros.h

Message ID 20230227154153.31080-5-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ß Feb. 27, 2023, 3:41 p.m. UTC
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>
---
 tools/firmware/hvmloader/util.h         | 3 ---
 tools/firmware/include/stddef.h         | 4 ++--
 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 --
 7 files changed, 9 insertions(+), 14 deletions(-)

Comments

Jan Beulich Feb. 27, 2023, 3:53 p.m. UTC | #1
On 27.02.2023 16:41, Juergen Gross wrote:
> --- a/tools/firmware/include/stddef.h
> +++ b/tools/firmware/include/stddef.h
> @@ -1,10 +1,10 @@
>  #ifndef _STDDEF_H_
>  #define _STDDEF_H_
>  
> +#include <xen-tools/common-macros.h>
> +
>  typedef __SIZE_TYPE__ size_t;
>  
>  #define NULL ((void*)0)
>  
> -#define offsetof(t, m) __builtin_offsetof(t, m)
> -
>  #endif

The C standard is pretty specific about what a header of this name
may or (in particular here) may not define. You add much more to the
name space than just the replacement offsetof(). If this was a
header used by an individual component, this might be fine. But this
header is meant to serve all components under firmware/ which care
to include it. At present that's hvmloader (which we control, so we
can arrange for it to be free of collisions) and rombios (which we
do not really control, and which people also may not build routinely
anymore).

Jan
Jürgen Groß March 6, 2023, 7:08 a.m. UTC | #2
On 27.02.23 16:53, Jan Beulich wrote:
> On 27.02.2023 16:41, Juergen Gross wrote:
>> --- a/tools/firmware/include/stddef.h
>> +++ b/tools/firmware/include/stddef.h
>> @@ -1,10 +1,10 @@
>>   #ifndef _STDDEF_H_
>>   #define _STDDEF_H_
>>   
>> +#include <xen-tools/common-macros.h>
>> +
>>   typedef __SIZE_TYPE__ size_t;
>>   
>>   #define NULL ((void*)0)
>>   
>> -#define offsetof(t, m) __builtin_offsetof(t, m)
>> -
>>   #endif
> 
> The C standard is pretty specific about what a header of this name
> may or (in particular here) may not define. You add much more to the
> name space than just the replacement offsetof(). If this was a
> header used by an individual component, this might be fine. But this
> header is meant to serve all components under firmware/ which care
> to include it. At present that's hvmloader (which we control, so we
> can arrange for it to be free of collisions) and rombios (which we
> do not really control, and which people also may not build routinely
> anymore).

Good point.

I have verified that the build is still succeeding without the modification
of tools/firmware/include/stddef.h, so I'll drop this hunk.


Juergen
diff mbox series

Patch

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/firmware/include/stddef.h b/tools/firmware/include/stddef.h
index c7f974608a..0afb426a6d 100644
--- a/tools/firmware/include/stddef.h
+++ b/tools/firmware/include/stddef.h
@@ -1,10 +1,10 @@ 
 #ifndef _STDDEF_H_
 #define _STDDEF_H_
 
+#include <xen-tools/common-macros.h>
+
 typedef __SIZE_TYPE__ size_t;
 
 #define NULL ((void*)0)
 
-#define offsetof(t, m) __builtin_offsetof(t, m)
-
 #endif
diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
index 3e73b33c91..15399e9b88 100644
--- a/tools/include/xen-tools/common-macros.h
+++ b/tools/include/xen-tools/common-macros.h
@@ -79,4 +79,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