2024-01-03 02:18:22

by Jiapeng Chong

[permalink] [raw]
Subject: [PATCH] platform/x86: hp-bioscfg: Remove useless else

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 <[email protected]>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7817
Signed-off-by: Jiapeng Chong <[email protected]>
---
.../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);
--
2.20.1.7.g153144c



2024-01-03 11:54:08

by Ilpo Järvinen

[permalink] [raw]
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 <[email protected]>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7817
> Signed-off-by: Jiapeng Chong <[email protected]>
> ---
> .../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/[email protected]/

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.