[Intel-gfx] [PATCH] sna: Initialize variable 'iter' to silence clang

Paul Menzel paulepanter at users.sourceforge.net
Thu Feb 28 09:45:37 CET 2013


Dear Sedat,


thank you for testing stuff with Clang!


Am Dienstag, den 26.02.2013, 15:15 +0100 schrieb Sedat Dilek:

Please always paste the error/warning messages so reviewers see exactly
what it has find.

> Signed-off-by: Sedat Dilek <sedat.dilek at gmail.com>
> ---
>  src/sna/sna_damage.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/sna/sna_damage.c b/src/sna/sna_damage.c
> index ab693af..53ed635 100644
> --- a/src/sna/sna_damage.c
> +++ b/src/sna/sna_damage.c
> @@ -410,7 +410,7 @@ static void __sna_damage_reduce(struct sna_damage *damage)
>  	int n, nboxes;
>  	BoxPtr boxes, free_boxes = NULL;
>  	pixman_region16_t *region = &damage->region;
> -	struct sna_damage_box *iter;
> +	struct sna_damage_box *iter = NULL;

Looking at the macro in `src/intel_list.h`

        /**
         * Loop through the list given by head and set pos to struct in the list.
         *
         * Example:
         * struct foo *iterator;
         * list_for_each_entry(iterator, &bar->list_of_foos, entry) {
         *      [modify iterator]
         * }
         *
         * This macro is not safe for node deletion. Use list_for_each_entry_safe
         * instead.
         *
         * @param pos Iterator variable of the type of the list elements.
         * @param head List head
         * @param member Member name of the struct list in the list elements.
         *
         */
        #define list_for_each_entry(pos, head, member)				\
            for (pos = __container_of((head)->next, pos, member);		\
        	 &pos->member != (head);					\
        	 pos = __container_of(pos->member.next, pos, member))

the iterator is definitely initialized in the for loop.

        pos = __container_of((head)->next

>  	assert(damage->mode != DAMAGE_ALL);
>  	assert(damage->dirty);
> @@ -1671,7 +1671,7 @@ void _sna_damage_debug_get_region(struct sna_damage *damage, RegionRec *r)
>  {
>  	int n, nboxes;
>  	BoxPtr boxes;
> -	struct sna_damage_box *iter;
> +	struct sna_damage_box *iter = NULL;
>  
>  	RegionCopy(r, &damage->region);
>  	if (!damage->dirty)

So in my opinion this patch should not be applied as it would prevent
the compiler to warn next time when there is really a code change where
this variable might be uninitialized.

By the way, does `gcc` warn about this?


Thanks,

Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20130228/8737cf00/attachment.sig>


More information about the Intel-gfx mailing list