[libdrm] Make sure the second argument to container_of() is initialized
diff mbox

Message ID 1470132535-3006-1-git-send-email-michel@daenzer.net
State New
Headers show

Commit Message

Michel Dänzer Aug. 2, 2016, 10:08 a.m. UTC
From: Michel Dänzer <michel.daenzer@amd.com>

Fixes warnings and miscompilation resulting in crashes with clang.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94249
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 amdgpu/amdgpu_vamgr.c |  2 +-
 util_double_list.h    | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Alex Deucher Aug. 2, 2016, 2:29 p.m. UTC | #1
On Tue, Aug 2, 2016 at 6:08 AM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> Fixes warnings and miscompilation resulting in crashes with clang.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94249
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  amdgpu/amdgpu_vamgr.c |  2 +-
>  util_double_list.h    | 10 +++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c
> index 8a707cb..3eaef70 100644
> --- a/amdgpu/amdgpu_vamgr.c
> +++ b/amdgpu/amdgpu_vamgr.c
> @@ -157,7 +157,7 @@ amdgpu_vamgr_find_va(struct amdgpu_bo_va_mgr *mgr, uint64_t size,
>  drm_private void
>  amdgpu_vamgr_free_va(struct amdgpu_bo_va_mgr *mgr, uint64_t va, uint64_t size)
>  {
> -       struct amdgpu_bo_va_hole *hole;
> +       struct amdgpu_bo_va_hole *hole = NULL;
>
>         if (va == AMDGPU_INVALID_VA_ADDRESS)
>                 return;
> diff --git a/util_double_list.h b/util_double_list.h
> index 5d01f52..fc32da5 100644
> --- a/util_double_list.h
> +++ b/util_double_list.h
> @@ -114,29 +114,29 @@ static inline void list_delinit(struct list_head *item)
>  #endif
>
>  #define LIST_FOR_EACH_ENTRY(pos, head, member)                         \
> -   for (pos = container_of((head)->next, pos, member);                 \
> +   for (pos = NULL, pos = container_of((head)->next, pos, member);     \
>         &pos->member != (head);                                         \
>         pos = container_of(pos->member.next, pos, member))
>
>  #define LIST_FOR_EACH_ENTRY_SAFE(pos, storage, head, member)   \
> -   for (pos = container_of((head)->next, pos, member),                 \
> +   for (pos = NULL, pos = container_of((head)->next, pos, member),     \
>         storage = container_of(pos->member.next, pos, member);  \
>         &pos->member != (head);                                         \
>         pos = storage, storage = container_of(storage->member.next, storage, member))
>
>  #define LIST_FOR_EACH_ENTRY_SAFE_REV(pos, storage, head, member)       \
> -   for (pos = container_of((head)->prev, pos, member),                 \
> +   for (pos = NULL, pos = container_of((head)->prev, pos, member),     \
>         storage = container_of(pos->member.prev, pos, member);          \
>         &pos->member != (head);                                         \
>         pos = storage, storage = container_of(storage->member.prev, storage, member))
>
>  #define LIST_FOR_EACH_ENTRY_FROM(pos, start, head, member)             \
> -   for (pos = container_of((start), pos, member);                      \
> +   for (pos = NULL, pos = container_of((start), pos, member);          \
>         &pos->member != (head);                                         \
>         pos = container_of(pos->member.next, pos, member))
>
>  #define LIST_FOR_EACH_ENTRY_FROM_REV(pos, start, head, member)         \
> -   for (pos = container_of((start), pos, member);                      \
> +   for (pos = NULL, pos = container_of((start), pos, member);          \
>         &pos->member != (head);                                         \
>         pos = container_of(pos->member.prev, pos, member))
>
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Nicolai Hähnle Aug. 2, 2016, 5:49 p.m. UTC | #2
On 02.08.2016 12:08, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> Fixes warnings and miscompilation resulting in crashes with clang.

How annoying. Seems like this would affect most (current and future) 
uses of container_of, and so it would be better to switch to the Linux 
kernel version of container_of, where the second parameter is the type...

Cheers,
Nicolai

>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94249
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>  amdgpu/amdgpu_vamgr.c |  2 +-
>  util_double_list.h    | 10 +++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c
> index 8a707cb..3eaef70 100644
> --- a/amdgpu/amdgpu_vamgr.c
> +++ b/amdgpu/amdgpu_vamgr.c
> @@ -157,7 +157,7 @@ amdgpu_vamgr_find_va(struct amdgpu_bo_va_mgr *mgr, uint64_t size,
>  drm_private void
>  amdgpu_vamgr_free_va(struct amdgpu_bo_va_mgr *mgr, uint64_t va, uint64_t size)
>  {
> -	struct amdgpu_bo_va_hole *hole;
> +	struct amdgpu_bo_va_hole *hole = NULL;
>
>  	if (va == AMDGPU_INVALID_VA_ADDRESS)
>  		return;
> diff --git a/util_double_list.h b/util_double_list.h
> index 5d01f52..fc32da5 100644
> --- a/util_double_list.h
> +++ b/util_double_list.h
> @@ -114,29 +114,29 @@ static inline void list_delinit(struct list_head *item)
>  #endif
>
>  #define LIST_FOR_EACH_ENTRY(pos, head, member)				\
> -   for (pos = container_of((head)->next, pos, member);			\
> +   for (pos = NULL, pos = container_of((head)->next, pos, member);	\
>  	&pos->member != (head);						\
>  	pos = container_of(pos->member.next, pos, member))
>
>  #define LIST_FOR_EACH_ENTRY_SAFE(pos, storage, head, member)	\
> -   for (pos = container_of((head)->next, pos, member),			\
> +   for (pos = NULL, pos = container_of((head)->next, pos, member),	\
>  	storage = container_of(pos->member.next, pos, member);	\
>  	&pos->member != (head);						\
>  	pos = storage, storage = container_of(storage->member.next, storage, member))
>
>  #define LIST_FOR_EACH_ENTRY_SAFE_REV(pos, storage, head, member)	\
> -   for (pos = container_of((head)->prev, pos, member),			\
> +   for (pos = NULL, pos = container_of((head)->prev, pos, member),	\
>  	storage = container_of(pos->member.prev, pos, member);		\
>  	&pos->member != (head);						\
>  	pos = storage, storage = container_of(storage->member.prev, storage, member))
>
>  #define LIST_FOR_EACH_ENTRY_FROM(pos, start, head, member)		\
> -   for (pos = container_of((start), pos, member);			\
> +   for (pos = NULL, pos = container_of((start), pos, member);		\
>  	&pos->member != (head);						\
>  	pos = container_of(pos->member.next, pos, member))
>
>  #define LIST_FOR_EACH_ENTRY_FROM_REV(pos, start, head, member)		\
> -   for (pos = container_of((start), pos, member);			\
> +   for (pos = NULL, pos = container_of((start), pos, member);		\
>  	&pos->member != (head);						\
>  	pos = container_of(pos->member.prev, pos, member))
>
>
Michel Dänzer Aug. 3, 2016, 8:56 a.m. UTC | #3
On 03.08.2016 02:49, Nicolai Hähnle wrote:
> On 02.08.2016 12:08, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> Fixes warnings and miscompilation resulting in crashes with clang.
> 
> How annoying. Seems like this would affect most (current and future)
> uses of container_of, and so it would be better to switch to the Linux
> kernel version of container_of, where the second parameter is the type...

Rob proposed http://hastebin.com/vefenavoje.coffee on IRC, using
typeof(). I'm fine with going for that instead of my patch, just FYI: My
patch was based on xserver's list.h, which works with compilers which
don't support typeof(). Not sure that's relevant for libdrm though.
Rob Clark Aug. 3, 2016, 11:20 a.m. UTC | #4
On Wed, Aug 3, 2016 at 4:56 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 03.08.2016 02:49, Nicolai Hähnle wrote:
>> On 02.08.2016 12:08, Michel Dänzer wrote:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> Fixes warnings and miscompilation resulting in crashes with clang.
>>
>> How annoying. Seems like this would affect most (current and future)
>> uses of container_of, and so it would be better to switch to the Linux
>> kernel version of container_of, where the second parameter is the type...
>
> Rob proposed http://hastebin.com/vefenavoje.coffee on IRC, using
> typeof(). I'm fine with going for that instead of my patch, just FYI: My
> patch was based on xserver's list.h, which works with compilers which
> don't support typeof(). Not sure that's relevant for libdrm though.

Actually I already went ahead and pushed that patch too.  Although
there may be other list implementations that need the same thing.

BR,
-R

Patch
diff mbox

diff --git a/amdgpu/amdgpu_vamgr.c b/amdgpu/amdgpu_vamgr.c
index 8a707cb..3eaef70 100644
--- a/amdgpu/amdgpu_vamgr.c
+++ b/amdgpu/amdgpu_vamgr.c
@@ -157,7 +157,7 @@  amdgpu_vamgr_find_va(struct amdgpu_bo_va_mgr *mgr, uint64_t size,
 drm_private void
 amdgpu_vamgr_free_va(struct amdgpu_bo_va_mgr *mgr, uint64_t va, uint64_t size)
 {
-	struct amdgpu_bo_va_hole *hole;
+	struct amdgpu_bo_va_hole *hole = NULL;
 
 	if (va == AMDGPU_INVALID_VA_ADDRESS)
 		return;
diff --git a/util_double_list.h b/util_double_list.h
index 5d01f52..fc32da5 100644
--- a/util_double_list.h
+++ b/util_double_list.h
@@ -114,29 +114,29 @@  static inline void list_delinit(struct list_head *item)
 #endif
 
 #define LIST_FOR_EACH_ENTRY(pos, head, member)				\
-   for (pos = container_of((head)->next, pos, member);			\
+   for (pos = NULL, pos = container_of((head)->next, pos, member);	\
 	&pos->member != (head);						\
 	pos = container_of(pos->member.next, pos, member))
 
 #define LIST_FOR_EACH_ENTRY_SAFE(pos, storage, head, member)	\
-   for (pos = container_of((head)->next, pos, member),			\
+   for (pos = NULL, pos = container_of((head)->next, pos, member),	\
 	storage = container_of(pos->member.next, pos, member);	\
 	&pos->member != (head);						\
 	pos = storage, storage = container_of(storage->member.next, storage, member))
 
 #define LIST_FOR_EACH_ENTRY_SAFE_REV(pos, storage, head, member)	\
-   for (pos = container_of((head)->prev, pos, member),			\
+   for (pos = NULL, pos = container_of((head)->prev, pos, member),	\
 	storage = container_of(pos->member.prev, pos, member);		\
 	&pos->member != (head);						\
 	pos = storage, storage = container_of(storage->member.prev, storage, member))
 
 #define LIST_FOR_EACH_ENTRY_FROM(pos, start, head, member)		\
-   for (pos = container_of((start), pos, member);			\
+   for (pos = NULL, pos = container_of((start), pos, member);		\
 	&pos->member != (head);						\
 	pos = container_of(pos->member.next, pos, member))
 
 #define LIST_FOR_EACH_ENTRY_FROM_REV(pos, start, head, member)		\
-   for (pos = container_of((start), pos, member);			\
+   for (pos = NULL, pos = container_of((start), pos, member);		\
 	&pos->member != (head);						\
 	pos = container_of(pos->member.prev, pos, member))