On Thu, 02 May 2019, Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> wrote: > Em qui, 2019-05-02 às 11:29 +0300, Jani Nikula escreveu: >> Using arithmetic operators with booleans is confusing. Switch to logical >> operators. >> >> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 4e7b8d..ef4992f 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -5094,7 +5094,7 @@ static void icl_update_tc_port_type(struct drm_i915_private *dev_priv, >> enum port port = intel_dig_port->base.port; >> enum tc_port_type old_type = intel_dig_port->tc_type; >> >> - WARN_ON(is_legacy + is_typec + is_tbt != 1); >> + WARN_ON(is_legacy || is_typec || !is_tbt); > > This changes the meaning. You're interpreting this as: > > WARN_ON(is_legacy + is_typec + (is_tbt != 1)) > > while the original intent of the code is to be: > > WARN_ON((is_legacy + is_typec + is_tbt) != 1) *blush* > and a quick check on operator precedence tables leads me to think the > original code is indeed correct. > > We're asserting exactly one of these bools enabled, so the logic > operation would be something like: > > WARN_ON((is_legacy && (is_typec || is_tbt)) || > (is_typec && (is_legacy || is_tbt)) || > (is_tbt && (is_legacy || is_typec)) || > (!is_legacy && !is_typec && !is_tbt)) > > I would still prefer the arithmetic operation. Agreed. I'll go hide under a rock. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
