Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 95150bdf authored by Jani Nikula's avatar Jani Nikula Committed by Daniel Vetter
Browse files

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: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarJani 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: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 373701b1
Loading
Loading
Loading
Loading
+6 −6
Original line number Original line Diff line number Diff line
@@ -288,7 +288,7 @@ struct i915_hotplug {
	list_for_each_entry(intel_plane,				\
	list_for_each_entry(intel_plane,				\
			    &(dev)->mode_config.plane_list,		\
			    &(dev)->mode_config.plane_list,		\
			    base.head)					\
			    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) \
#define for_each_intel_crtc(dev, intel_crtc) \
	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
	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) \
#define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
	list_for_each_entry((intel_encoder), &(dev)->mode_config.encoder_list, base.head) \
	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) \
#define for_each_connector_on_encoder(dev, __encoder, intel_connector) \
	list_for_each_entry((intel_connector), &(dev)->mode_config.connector_list, base.head) \
	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)				\
#define for_each_power_domain(domain, mask)				\
	for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)	\
	for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)	\
		if ((1 << (domain)) & (mask))
		for_each_if ((1 << (domain)) & (mask))


struct drm_i915_private;
struct drm_i915_private;
struct i915_mm_struct;
struct i915_mm_struct;
@@ -734,7 +734,7 @@ struct intel_uncore {
	for ((i__) = 0, (domain__) = &(dev_priv__)->uncore.fw_domain[0]; \
	for ((i__) = 0, (domain__) = &(dev_priv__)->uncore.fw_domain[0]; \
	     (i__) < FW_DOMAIN_ID_COUNT; \
	     (i__) < FW_DOMAIN_ID_COUNT; \
	     (i__)++, (domain__) = &(dev_priv__)->uncore.fw_domain[i__]) \
	     (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__) \
#define for_each_fw_domain(domain__, dev_priv__, i__) \
	for_each_fw_domain_mask(domain__, FORCEWAKE_ALL, 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 */
/* Iterate over initialised rings */
#define for_each_ring(ring__, dev_priv__, i__) \
#define for_each_ring(ring__, dev_priv__, i__) \
	for ((i__) = 0; (i__) < I915_NUM_RINGS; (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 {
enum hdmi_force_audio {
	HDMI_AUDIO_OFF_DVI = -2,	/* no aux data for HDMI-DVI converter */
	HDMI_AUDIO_OFF_DVI = -2,	/* no aux data for HDMI-DVI converter */
+1 −1
Original line number Original line Diff line number Diff line
@@ -12281,7 +12281,7 @@ static bool intel_fuzzy_clock_check(int clock1, int clock2)
	list_for_each_entry((intel_crtc), \
	list_for_each_entry((intel_crtc), \
			    &(dev)->mode_config.crtc_list, \
			    &(dev)->mode_config.crtc_list, \
			    base.head) \
			    base.head) \
		if (mask & (1 <<(intel_crtc)->pipe))
		for_each_if (mask & (1 <<(intel_crtc)->pipe))


static bool
static bool
intel_compare_m_n(unsigned int m, unsigned int n,
intel_compare_m_n(unsigned int m, unsigned int n,
+1 −1
Original line number Original line Diff line number Diff line
@@ -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) \
#define for_each_dsi_port(__port, __ports_mask) \
	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
	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)
static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
{
{
+2 −2
Original line number Original line Diff line number Diff line
@@ -57,13 +57,13 @@
	     i < (power_domains)->power_well_count &&			\
	     i < (power_domains)->power_well_count &&			\
		 ((power_well) = &(power_domains)->power_wells[i]);	\
		 ((power_well) = &(power_domains)->power_wells[i]);	\
	     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) \
#define for_each_power_well_rev(i, power_well, domain_mask, power_domains) \
	for (i = (power_domains)->power_well_count - 1;			 \
	for (i = (power_domains)->power_well_count - 1;			 \
	     i >= 0 && ((power_well) = &(power_domains)->power_wells[i]);\
	     i >= 0 && ((power_well) = &(power_domains)->power_wells[i]);\
	     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,
bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
				    int power_well_id);
				    int power_well_id);