linux-kernel - Re: [PATCH 1/6] drm/ast: Remove redundant else in atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <325bf106-2b5c-4b13-be2a-e05a3c559bde@suse.de>
Date: Tue, 25 Feb 2025 12:49:20 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Aditya Garg <gargaditya08@...e.com>,
 "airlied@...hat.com" <airlied@...hat.com>,
 "jfalempe@...hat.com" <jfalempe@...hat.com>,
 "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
 "mripard@...nel.org" <mripard@...nel.org>,
 "airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch"
 <simona@...ll.ch>, "kraxel@...hat.com" <kraxel@...hat.com>,
 "javierm@...hat.com" <javierm@...hat.com>
Cc: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>,
 "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH 1/6] drm/ast: Remove redundant else in atomic_check

Hi

Am 25.02.25 um 12:06 schrieb Aditya Garg:
> From: Aditya Garg <gargaditya08@...e.com>
>
> Remove the redundant else statement from atomic_check since the previous if
> statement was returning if true.
>
> Signed-off-by: Aditya Garg <gargaditya08@...e.com>
> ---
>   drivers/gpu/drm/ast/ast_mode.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 9d5321c81..3817d1e4c 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -610,9 +610,10 @@ static int ast_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   						  DRM_PLANE_NO_SCALING,
>   						  DRM_PLANE_NO_SCALING,
>   						  false, true);
> -	if (ret) {
> +	if (ret)
>   		return ret;
> -	} else if (!new_plane_state->visible) {
> +
> +	if (!new_plane_state->visible) {

I've seen this posted before.

The reason why there is an 'else' branch here is that both branches 
handle the state returned by the function call above, 
drm_atomic_helper_check_plane_state(). First it does an error check, and 
then it tests for >visible. In both cases, the plane's atomic_check 
should return. And only if we have a valid and visible plane, we do the 
actual checks on the plane. Conceptually, these if-else cases belong 
together and signal an early-out from the call.

I'd prefer to keep the drivers as they are.

Best regards
Thomas


>   		if (drm_WARN_ON(dev, new_plane_state->crtc)) /* cannot legally happen */
>   			return -EINVAL;
>   		else

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)