drm: 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 drm_for_each_plane_mask(plane, dev, plane_mask) \
         list_for_each_entry((plane), &(dev)->mode_config.plane_list, head) \
                 if ((plane_mask) & (1 << drm_plane_index(plane)))

If this is used in context:

	if (condition)
		drm_for_each_plane_mask(plane, dev, plane_mask);
	else
		foo();

foo() will be called for each plane *not* in plane_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.

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

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0b921ae06cd83585e1d1cf2adb6baf665f203013..30d4a5a495e25661c9d1e2498b26dadf40ad9a17 100644 (file)
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1111,4 +1111,7 @@ static __inline__ bool drm_can_sleep(void)
        return true;
 }
 
+/* helper for handling conditionals in various for_each macros */
+#define for_each_if(condition) if (!(condition)) {} else
+
 #endif

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 4b74c97d297a3e8f24a50de057cb4db79750f39c..d8576ac55693da65c73cb7e66f38108e47e46242 100644 (file)
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -149,7 +149,7 @@ int __must_check drm_atomic_async_commit(struct drm_atomic_state *state);
             ((connector) = (state)->connectors[__i],                   \
             (connector_state) = (state)->connector_states[__i], 1);    \
             (__i)++)                                                   \
-               if (connector)
+               for_each_if (connector)
 
 #define for_each_crtc_in_state(state, crtc, crtc_state, __i)   \
        for ((__i) = 0;                                         \
@@ -157,7 +157,7 @@ int __must_check drm_atomic_async_commit(struct drm_atomic_state *state);
             ((crtc) = (state)->crtcs[__i],                     \
             (crtc_state) = (state)->crtc_states[__i], 1);      \
             (__i)++)                                           \
-               if (crtc_state)
+               for_each_if (crtc_state)
 
 #define for_each_plane_in_state(state, plane, plane_state, __i)                \
        for ((__i) = 0;                                                 \
@@ -165,7 +165,7 @@ int __must_check drm_atomic_async_commit(struct drm_atomic_state *state);
             ((plane) = (state)->planes[__i],                           \
             (plane_state) = (state)->plane_states[__i], 1);            \
             (__i)++)                                                   \
-               if (plane_state)
+               for_each_if (plane_state)
 static inline bool
 drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state)
 {

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 173535a35d650c501b10997a2b75f8629a9116ed..4765df3310025fa120d432bd8b8906740256b313 100644 (file)
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1170,7 +1170,7 @@ struct drm_mode_config {
  */
 #define drm_for_each_plane_mask(plane, dev, plane_mask) \
        list_for_each_entry((plane), &(dev)->mode_config.plane_list, head) \
-               if ((plane_mask) & (1 << drm_plane_index(plane)))
+               for_each_if ((plane_mask) & (1 << drm_plane_index(plane)))
 
 
 #define obj_to_crtc(x) container_of(x, struct drm_crtc, base)
@@ -1547,7 +1547,7 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
 /* Plane list iterator for legacy (overlay only) planes. */
 #define drm_for_each_legacy_plane(plane, dev) \
        list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
-               if (plane->type == DRM_PLANE_TYPE_OVERLAY)
+               for_each_if (plane->type == DRM_PLANE_TYPE_OVERLAY)
 
 #define drm_for_each_plane(plane, dev) \
        list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)