summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNeil Roberts <neil@linux.intel.com>2014-02-04 14:21:48 (GMT)
committerKristian Høgsberg <krh@bitplanet.net>2014-02-06 01:21:43 (GMT)
commita18e34417ba3fefeb81d891235e8ebf394a20a74 (patch)
tree2be43530fb686244c7c7b535809935bbb7d48665
parentbab7f46d83fc7890202966dc62d61d95480d7709 (diff)
downloadwayland-a18e34417ba3fefeb81d891235e8ebf394a20a74.tar.gz
wayland-a18e34417ba3fefeb81d891235e8ebf394a20a74.tar.xz
Don't deref the sample pointer in the wl_container_of macro
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.
-rw-r--r--src/wayland-server.c2
-rw-r--r--src/wayland-server.h2
-rw-r--r--src/wayland-util.h30
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, \