[PATCH] Don't deref the sample pointer in the wl_container_of macro

[Neil Roberts]

The previous implementation of the wl_container_of macro was
dereferencing the sample pointer in order to get an address of the
member to calculate the offset. Ideally this shouldn't cause any
problems because the dereference doesn't actually cause the address to
be read from so it shouldn't matter if the pointer is uninitialised.
However this is probably technically invalid and could cause undefined
behavior. Clang appears to take advantage of this undefined behavior
and doesn't bother doing the subtraction. It also gives a warning when
it does this.

The documentation for wl_container_of implies that it should only be
given an initialised pointer and if that is done then there is no
problem with clang. However this is quite easy to forget and doesn't
cause any problems or warnings with gcc so it's quite easy to
accidentally break clang.

To fix the problem this changes the macro to use pointer -
offsetof(__typeof__(sample), member) so that it doesn't need to deref
the sample pointer. This does however require that the __typeof__
operator is supported by the compiler. In practice we probably only
care about gcc and clang and both of these happily support the
operator.

The previous implementation was also using __typeof__ but it had a
fallback path avoiding it when the operator isn't available. The
fallback effectively has undefined behaviour and it is targetting
unknown compilers so it is probably not a good idea to leave it in.
Instead, this patch just removes it. If someone finds a compiler that
doesn't have __typeof__ but does work with the old implementation then
maybe they could add it back in as a special case.

This patch removes the initialisation anywhere where the sample
pointer was being unitialised before using wl_container_of. The
documentation for the macro has also been updated to specify that this
is OK.
---
 src/wayland-server.c |  2 +-
 src/wayland-server.h |  2 +-
 src/wayland-util.h   | 30 +++++++++++++-----------------
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/wayland-server.c b/src/wayland-server.c
index 489b99d..294497d 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -571,7 +571,7 @@ wl_resource_get_link(struct wl_resource *resource)
 WL_EXPORT struct wl_resource *
 wl_resource_from_link(struct wl_list *link)
 {
-	struct wl_resource *resource = NULL;
+	struct wl_resource *resource;
 
 	return wl_container_of(link, resource, link);
 }
diff --git a/src/wayland-server.h b/src/wayland-server.h
index 3fcaaf6..7fc5b47 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -162,7 +162,7 @@ wl_client_post_no_memory(struct wl_client *client);
  * \code
  * void your_listener(struct wl_listener *listener, void *data)
  * {
- * 	struct your_data *data = NULL;
+ * 	struct your_data *data;
  *
  * 	your_data = wl_container_of(listener, data, your_member_name);
  * }
diff --git a/src/wayland-util.h b/src/wayland-util.h
index 68d91e2..57764d9 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -135,7 +135,7 @@ void wl_list_insert_list(struct wl_list *list, struct wl_list *other);
  *
  * void example_container_destroy(struct wl_listener *listener, void *data)
  * {
- *     struct example_container *ctr = NULL;
+ *     struct example_container *ctr;
  *
  *     ctr = wl_container_of(listener, ctr, destroy_listener);
  *     \comment{destroy ctr...}
@@ -144,44 +144,40 @@ void wl_list_insert_list(struct wl_list *list, struct wl_list *other);
  *
  * \param ptr A valid pointer to the contained item.
  *
- * \param sample A pointer to the type of content that the list item stores.
- * Sample does not need be a valid pointer; a null pointer will suffice.
+ * \param sample A pointer to the type of content that the list item
+ * stores. Sample does not need be a valid pointer; a null or
+ * an uninitialised pointer will suffice.
  *
  * \param member The named location of ptr within the sample type.
  *
  * \return The container for the specified pointer.
  */
-#ifdef __GNUC__
 #define wl_container_of(ptr, sample, member)				\
-	(__typeof__(sample))((char *)(ptr)	-			\
-		 ((char *)&(sample)->member - (char *)(sample)))
-#else
-#define wl_container_of(ptr, sample, member)				\
-	(void *)((char *)(ptr)	-				        \
-		 ((char *)&(sample)->member - (char *)(sample)))
-#endif
+	(__typeof__(sample))((char *)(ptr) -				\
+			     offsetof(__typeof__(*sample), member))
+/* If the above macro causes problems on your compiler you might be
+ * able to find an alternative name for the non-standard __typeof__
+ * operator and add a special case here */
 
 #define wl_list_for_each(pos, head, member)				\
-	for (pos = 0, pos = wl_container_of((head)->next, pos, member);	\
+	for (pos = wl_container_of((head)->next, pos, member);	\
 	     &pos->member != (head);					\
 	     pos = wl_container_of(pos->member.next, pos, member))
 
 #define wl_list_for_each_safe(pos, tmp, head, member)			\
-	for (pos = 0, tmp = 0, 						\
-	     pos = wl_container_of((head)->next, pos, member),		\
+	for (pos = wl_container_of((head)->next, pos, member),		\
 	     tmp = wl_container_of((pos)->member.next, tmp, member);	\
 	     &pos->member != (head);					\
 	     pos = tmp,							\
 	     tmp = wl_container_of(pos->member.next, tmp, member))
 
 #define wl_list_for_each_reverse(pos, head, member)			\
-	for (pos = 0, pos = wl_container_of((head)->prev, pos, member);	\
+	for (pos = wl_container_of((head)->prev, pos, member);	\
 	     &pos->member != (head);					\
 	     pos = wl_container_of(pos->member.prev, pos, member))
 
 #define wl_list_for_each_reverse_safe(pos, tmp, head, member)		\
-	for (pos = 0, tmp = 0, 						\
-	     pos = wl_container_of((head)->prev, pos, member),	\
+	for (pos = wl_container_of((head)->prev, pos, member),	\
 	     tmp = wl_container_of((pos)->member.prev, tmp, member);	\
 	     &pos->member != (head);					\
 	     pos = tmp,							\
-- 
1.8.4.2