[PATCH v2 1/1] list: Use offsetof() and typeof() to determine member offsets within a structure

[Peter Hutterer]

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 <jeremyhu at apple.com>
> ---
>  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 <peter.hutterer at who-t.net> 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
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>