On Mon, 16 Mar 2015, Ander Conselvan de Oliveira 
<ander.conselvan.de.olive...@intel.com> wrote:
> So that the compiler warns if we get something wrong.
>
> Signed-off-by: Ander Conselvan de Oliveira 
> <ander.conselvan.de.olive...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 39 ++++++++++++++++++++++++++-------------
>  1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ae4ba6a..871d77a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -232,30 +232,43 @@ enum hpd_pin {
>            (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];        \
>            (__s)++)
>  
> -#define for_each_crtc(dev, crtc) \
> +#define type_check(variable, type)                           \
> +     {                                                       \
> +             typeof(variable) __dummy_ptr = NULL;            \
> +             const type *__type_check = __dummy_ptr;         \
> +             (void) __type_check;                            \
> +     }
> +
> +#define for_each_crtc(dev, crtc)                             \
> +     type_check(crtc, struct drm_crtc)                       \
>       list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)

if (foo)
        for_each_crtc(dev, crtc);

will do type_check if foo, but list_for_each_entry unconditionally.

I think this is too fragile.

I'm already pretty anxious about our loop macros with an if in them
blowing up due to the dangling else:

        if (foo)
                list_for_each_encoder_on_crtc(dev, crtc, intel_encoder)
                        bar();
        else
                baz();

where baz() gets called for each encoder *not* on crtc, instead of once
for !foo.


BR,
Jani.



>  
> -#define for_each_intel_crtc(dev, intel_crtc) \
> -     list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
> +#define for_each_intel_crtc(dev, crtc)                               \
> +     type_check(crtc, struct intel_crtc)                     \
> +     list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
>  
> -#define for_each_intel_encoder(dev, intel_encoder)           \
> -     list_for_each_entry(intel_encoder,                      \
> +#define for_each_intel_encoder(dev, encoder)                 \
> +     type_check(encoder, struct intel_encoder)               \
> +     list_for_each_entry(encoder,                            \
>                           &(dev)->mode_config.encoder_list,   \
>                           base.head)
>  
> -#define for_each_intel_connector(dev, intel_connector)               \
> -     list_for_each_entry(intel_connector,                    \
> +#define for_each_intel_connector(dev, connector)             \
> +     type_check(connector, struct intel_connector);          \
> +     list_for_each_entry(connector,                          \
>                           &dev->mode_config.connector_list,   \
>                           base.head)
>  
>  
> -#define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
> -     list_for_each_entry((intel_encoder), &(dev)->mode_config.encoder_list, 
> base.head) \
> -             if ((intel_encoder)->base.crtc == (__crtc))
> +#define for_each_encoder_on_crtc(dev, __crtc, encoder)               \
> +     type_check(encoder, struct intel_encoder)               \
> +     list_for_each_entry((encoder), &(dev)->mode_config.encoder_list, 
> base.head) \
> +             if ((encoder)->base.crtc == (__crtc))
>  
> -#define for_each_connector_on_encoder(dev, __encoder, intel_connector) \
> -     list_for_each_entry((intel_connector), 
> &(dev)->mode_config.connector_list, base.head) \
> -             if ((intel_connector)->base.encoder == (__encoder))
> +#define for_each_connector_on_encoder(dev, __encoder, connector) \
> +     type_check(connector, struct intel_connector)           \
> +     list_for_each_entry((connector), &(dev)->mode_config.connector_list, 
> base.head) \
> +             if ((connector)->base.encoder == (__encoder))
>  
>  #define for_each_power_domain(domain, mask)                          \
>       for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)     \
> -- 
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to