Re: [PATCH] drm/i915/dp: use logical operators with boolean type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)

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.    

>  
>  	if (is_legacy)
>  		intel_dig_port->tc_type = TC_PORT_LEGACY;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux