drm/i915: fix potential dangling else problems in for_each_ macros

We have serious dangling else bugs waiting to happen in our for_each_
style macros with ifs. Consider, for example,

 #define for_each_power_domain(domain, mask)                         \
         for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \
                 if ((1 << (domain)) & (mask))

If this is used in context:

	if (condition)
		for_each_power_domain(domain, mask);
	else
		foo();

foo() will be called for each domain *not* in mask, if condition holds,
and not at all if condition doesn't hold.

Fix this by reversing the conditions in the macros, and adding an else
branch for the "for each" block, so that other if/else blocks can't
interfere. Provide a "for_each_if" helper macro to make it easier to get
this right.

v2: move for_each_if to drmP.h in a separate patch.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1448392916-2281-2-git-send-email-jani.nikula@intel.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 95bb27de774f8fb2c5bb8cc83567a45eb83a8e43..fd88060b2596f2c07b057ac657aac3011f773c92 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -288,7 +288,7 @@ struct i915_hotplug {
        list_for_each_entry(intel_plane,                                \
                            &(dev)->mode_config.plane_list,             \
                            base.head)                                  \
-               if ((intel_plane)->pipe == (intel_crtc)->pipe)
+               for_each_if ((intel_plane)->pipe == (intel_crtc)->pipe)
 
 #define for_each_intel_crtc(dev, intel_crtc) \
        list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
@@ -305,15 +305,15 @@ struct i915_hotplug {
 
 #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))
+               for_each_if ((intel_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))
+               for_each_if ((intel_connector)->base.encoder == (__encoder))
 
 #define for_each_power_domain(domain, mask)                            \
        for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)     \
-               if ((1 << (domain)) & (mask))
+               for_each_if ((1 << (domain)) & (mask))
 
 struct drm_i915_private;
 struct i915_mm_struct;
@@ -734,7 +734,7 @@ struct intel_uncore {
        for ((i__) = 0, (domain__) = &(dev_priv__)->uncore.fw_domain[0]; \
             (i__) < FW_DOMAIN_ID_COUNT; \
             (i__)++, (domain__) = &(dev_priv__)->uncore.fw_domain[i__]) \
-               if (((mask__) & (dev_priv__)->uncore.fw_domains) & (1 << (i__)))
+               for_each_if (((mask__) & (dev_priv__)->uncore.fw_domains) & (1 << (i__)))
 
 #define for_each_fw_domain(domain__, dev_priv__, i__) \
        for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, dev_priv__, i__)
@@ -1979,7 +1979,7 @@ static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
 /* Iterate over initialised rings */
 #define for_each_ring(ring__, dev_priv__, i__) \
        for ((i__) = 0; (i__) < I915_NUM_RINGS; (i__)++) \
-               if (((ring__) = &(dev_priv__)->ring[(i__)]), intel_ring_initialized((ring__)))
+               for_each_if ((((ring__) = &(dev_priv__)->ring[(i__)]), intel_ring_initialized((ring__))))
 
 enum hdmi_force_audio {
        HDMI_AUDIO_OFF_DVI = -2,        /* no aux data for HDMI-DVI converter */

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f02a099872fdf3dbfd5ccdcb36a6d862ca17778..bea7f3aef2b0359a0233386a00bb135ac62e4192 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12281,7 +12281,7 @@ static bool intel_fuzzy_clock_check(int clock1, int clock2)
        list_for_each_entry((intel_crtc), \
                            &(dev)->mode_config.crtc_list, \
                            base.head) \
-               if (mask & (1 <<(intel_crtc)->pipe))
+               for_each_if (mask & (1 <<(intel_crtc)->pipe))
 
 static bool
 intel_compare_m_n(unsigned int m, unsigned int n,

diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e6cb252399417cb8d882d6ff932b2f716ccd99c5..02551ff228c22e32751b2e34803668432e7c6d64 100644 (file)
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -117,7 +117,7 @@ static inline struct intel_dsi_host *to_intel_dsi_host(struct mipi_dsi_host *h)
 
 #define for_each_dsi_port(__port, __ports_mask) \
        for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)  \
-               if ((__ports_mask) & (1 << (__port)))
+               for_each_if ((__ports_mask) & (1 << (__port)))
 
 static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 {

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index d89c1d0aa1b74793a4328c861c312a91709092e7..13f14208d8aa21c59aa9e737b2a90c3b44c3b8a8 100644 (file)
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -57,13 +57,13 @@
             i < (power_domains)->power_well_count &&                   \
                 ((power_well) = &(power_domains)->power_wells[i]);     \
             i++)                                                       \
-               if ((power_well)->domains & (domain_mask))
+               for_each_if ((power_well)->domains & (domain_mask))
 
 #define for_each_power_well_rev(i, power_well, domain_mask, power_domains) \
        for (i = (power_domains)->power_well_count - 1;                  \
             i >= 0 && ((power_well) = &(power_domains)->power_wells[i]);\
             i--)                                                        \
-               if ((power_well)->domains & (domain_mask))
+               for_each_if ((power_well)->domains & (domain_mask))
 
 bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
                                    int power_well_id);