On Wed, Aug 29, 2012 at 09:22:16AM -0700, Jeremy Huddleston Sequoia wrote: > Some compilers have difficulty with the previous implementation which > relies on undefined behavior according to the C standard. Using > offsetof() from <stddef.h> (which most likely just uses > __builtin_offsetof on modern compilers) allows us to accomplish this > without ambiguity. > > This fix also requires support for typeof(). If your compiler does not > support typeof(), then the old implementation will be used. If you see > failures in test/list, please try a more modern compiler. > > v2: Added fallback if typeof() is not present. > > Signed-off-by: Jeremy Huddleston Sequoia <[email protected]> > --- > configure.ac | 1 + > include/dix-config.h.in | 3 +++ > include/list.h | 25 +++++++++++++++++++++---- > 3 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/configure.ac b/configure.ac > index abfe727..ab89027 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -137,6 +137,7 @@ AC_CHECK_HEADERS([fcntl.h stdlib.h string.h unistd.h > dlfcn.h stropts.h fnmatch.h > > dnl Checks for typedefs, structures, and compiler characteristics. > AC_C_CONST > +AC_C_TYPEOF > AC_C_BIGENDIAN([ENDIAN="X_BIG_ENDIAN"], [ENDIAN="X_LITTLE_ENDIAN"]) > > AC_CHECK_SIZEOF([unsigned long]) > diff --git a/include/dix-config.h.in b/include/dix-config.h.in > index 77681a9..578f249 100644 > --- a/include/dix-config.h.in > +++ b/include/dix-config.h.in > @@ -423,6 +423,9 @@ > /* Define to 64-bit byteswap macro */ > #undef bswap_64 > > +/* Define to 1 if typeof works with your compiler. */ > +#undef HAVE_TYPEOF > + > /* The compiler supported TLS storage class, prefering initial-exec if > tls_model is supported */ > #undef TLS > > diff --git a/include/list.h b/include/list.h > index d54a207..edf10ba 100644 > --- a/include/list.h > +++ b/include/list.h > @@ -26,6 +26,12 @@ > #ifndef _XORG_LIST_H_ > #define _XORG_LIST_H_ > > +#ifdef HAVE_DIX_CONFIG_H > +#include "dix-config.h" > +#endif
shouldn't this be included by whatever requires the list header? a git grep for HAVE_DIX_CONFIG_H in include only shows two headers using it, and it's likely neither of them should. Reviewed-by: Peter Hutterer <[email protected]> to the rest of the patch Cheers, Peter > + > +#include <stddef.h> /* offsetof() */ > + > /** > * @file Classic doubly-link circular list implementation. > * For real usage examples of the linked list, see the file test/list.c > @@ -232,7 +238,7 @@ xorg_list_is_empty(struct xorg_list *head) > */ > #ifndef container_of > #define container_of(ptr, type, member) \ > - (type *)((char *)(ptr) - (char *) &((type *)0)->member) > + (type *)((char *)(ptr) - offsetof(type, member)) > #endif > > /** > @@ -271,9 +277,20 @@ xorg_list_is_empty(struct xorg_list *head) > #define xorg_list_last_entry(ptr, type, member) \ > xorg_list_entry((ptr)->prev, type, member) > > -#define __container_of(ptr, sample, member) \ > - (void *)((char *)(ptr) \ > - - ((char *)&(sample)->member - (char *)(sample))) > +#ifdef HAVE_TYPEOF > +#define __container_of(ptr, sample, member) \ > + container_of(ptr, typeof(*sample), member) > +#else > +/* This implementation of __container_of has undefined behavior according > + * to the C standard, but it works in many cases. If your compiler doesn't > + * support typeof() and fails with this implementation, please try a newer > + * compiler. > + */ > +#define __container_of(ptr, sample, member) \ > + (void *)((char *)(ptr) \ > + - ((char *)&(sample)->member - (char *)(sample))) > +#endif > + > /** > * Loop through the list given by head and set pos to struct in the list. > * > -- > 1.7.11.5 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
