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]
Date: Wed, 3 Jan 2024 13:53:48 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>
cc: jorge.lopez2@...com, Hans de Goede <hdegoede@...hat.com>, 
    platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>, 
    Abaci Robot <abaci@...ux.alibaba.com>
Subject: Re: [PATCH] platform/x86: hp-bioscfg: Remove useless else

On Wed, 3 Jan 2024, Jiapeng Chong wrote:

> The assignment of the else and if branches is the same, so the else
> here is redundant, so we remove it.
> 
> ./drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c:544:3-5: WARNING: possible condition with no effect (if == else).
> 
> Reported-by: Abaci Robot <abaci@...ux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7817
> Signed-off-by: Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>
> ---
>  .../platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> index f7efe217a4bb..18c60a847842 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -540,14 +540,8 @@ void hp_exit_password_attributes(void)
>  		struct kobject *attr_name_kobj =
>  			bioscfg_drv.password_data[instance_id].attr_name_kobj;
>  
> -		if (attr_name_kobj) {
> -			if (!strcmp(attr_name_kobj->name, SETUP_PASSWD))
> -				sysfs_remove_group(attr_name_kobj,
> -						   &password_attr_group);
> -			else
> -				sysfs_remove_group(attr_name_kobj,
> -						   &password_attr_group);
> -		}
> +		if (attr_name_kobj)
> +			sysfs_remove_group(attr_name_kobj, &password_attr_group);
>  	}
>  	bioscfg_drv.password_instances_count = 0;
>  	kfree(bioscfg_drv.password_data);

When doing something based on a robot finding, please take a look at 
the related code and _think_(!) instead of just hitting send button. If 
you'd have done that, you'd have submitted a patch that cleans up the 
other (create) cases too, not just the one which your robot flagged.

I think this is the second time I've said this about the very same code 
construct to somebody which, disappointingly, turns out to be you:

https://lore.kernel.org/platform-driver-x86/2ec499b-c37e-0a9-c163-2a1591b56029@linux.intel.com/

Again I get an incomplete patch into my inbox because the previous review 
did not lead into an updated v2 patch. Please do not submit this patch 
again unless you addressed my review feedback.

-- 
 i.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ