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] [day] [month] [year] [list]

Message-ID:
 <PN3PR01MB959759051DDE76D2EBC58FD5B8C32@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
Date: Tue, 25 Feb 2025 11:50:57 +0000
From: Aditya Garg <gargaditya08@...e.com>
To: Thomas Zimmermann <tzimmermann@...e.de>
CC: "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>, "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



> On 25 Feb 2025, at 5:19 PM, Thomas Zimmermann <tzimmermann@...e.de> wrote:
> 
> 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.

Alright, should this be kept in the appletbdrm driver as well then?