Skip to content

Commit

Permalink
drm: fix potential dangling else problems in for_each_ macros
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jani Nikula authored and Daniel Vetter committed Nov 25, 2015
1 parent ed293f7 commit 373701b
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
3 changes: 3 additions & 0 deletions include/drm/drmP.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions include/drm/drm_atomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,23 +149,23 @@ 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; \
(__i) < (state)->dev->mode_config.num_crtc && \
((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; \
(__i) < (state)->dev->mode_config.num_total_plane && \
((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)
{
Expand Down
4 changes: 2 additions & 2 deletions include/drm/drm_crtc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 373701b

Please sign in to comment.