* [PATCH] platform/x86: hp-bioscfg: Remove useless else
@ 2023-08-11 6:29 Jiapeng Chong
2023-08-11 8:04 ` Ilpo Järvinen
2023-08-11 15:42 ` Lopez, Jorge A (Security)
0 siblings, 2 replies; 6+ messages in thread
From: Jiapeng Chong @ 2023-08-11 6:29 UTC (permalink / raw)
To: jorge.lopez2
Cc: hdegoede, markgross, platform-driver-x86, linux-kernel,
Jiapeng Chong, Abaci Robot
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:545:3-5: WARNING: possible condition with no effect (if == else).
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.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 03d0188804ba..771e554182dc 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -541,14 +541,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] platform/x86: hp-bioscfg: Remove useless else
2023-08-11 6:29 [PATCH] platform/x86: hp-bioscfg: Remove useless else Jiapeng Chong
@ 2023-08-11 8:04 ` Ilpo Järvinen
2023-08-11 15:41 ` Lopez, Jorge A (Security)
2023-08-11 15:42 ` Lopez, Jorge A (Security)
1 sibling, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2023-08-11 8:04 UTC (permalink / raw)
To: Jiapeng Chong
Cc: jorge.lopez2, hdegoede, markgross, platform-driver-x86, LKML,
Abaci Robot
On Fri, 11 Aug 2023, 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:545:3-5: WARNING: possible condition with no effect (if == else).
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.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 03d0188804ba..771e554182dc 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -541,14 +541,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);
While this is a valid observation, I wonder why you didn't change the
create side too?
I hope it's not because you never took a look to understand the
SETUP_PASSWD related code, it'd be pretty impossible to not notice it.
This could have just as well have been a bug with inconsitent pairing
against create side, which is why reports from automated tools must not be
implemented blindly but the related code must be understood first.
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] platform/x86: hp-bioscfg: Remove useless else
2023-08-11 8:04 ` Ilpo Järvinen
@ 2023-08-11 15:41 ` Lopez, Jorge A (Security)
0 siblings, 0 replies; 6+ messages in thread
From: Lopez, Jorge A (Security) @ 2023-08-11 15:41 UTC (permalink / raw)
To: Ilpo Järvinen, Jiapeng Chong
Cc: hdegoede@redhat.com, markgross@kernel.org,
platform-driver-x86@vger.kernel.org, LKML, Abaci Robot
The redundant code is a remnant code from test phase.
I concur with your changes which were approved in a separate email.
Regards,
Jorge Lopez
HP Inc
"Once you stop learning, you start dying"
Albert Einstein
> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Friday, August 11, 2023 3:04 AM
> To: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> Cc: Lopez, Jorge A (Security) <jorge.lopez2@hp.com>;
> hdegoede@redhat.com; markgross@kernel.org; platform-driver-
> x86@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>; Abaci Robot
> <abaci@linux.alibaba.com>
> Subject: Re: [PATCH] platform/x86: hp-bioscfg: Remove useless else
>
> CAUTION: External Email
>
> On Fri, 11 Aug 2023, 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:545:3-5:
> WARNING: possible condition with no effect (if == else).
> >
> > Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.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 03d0188804ba..771e554182dc 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > @@ -541,14 +541,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);
>
> While this is a valid observation, I wonder why you didn't change the create
> side too?
>
> I hope it's not because you never took a look to understand the
> SETUP_PASSWD related code, it'd be pretty impossible to not notice it.
> This could have just as well have been a bug with inconsitent pairing against
> create side, which is why reports from automated tools must not be
> implemented blindly but the related code must be understood first.
>
> --
> i.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] platform/x86: hp-bioscfg: Remove useless else
2023-08-11 6:29 [PATCH] platform/x86: hp-bioscfg: Remove useless else Jiapeng Chong
2023-08-11 8:04 ` Ilpo Järvinen
@ 2023-08-11 15:42 ` Lopez, Jorge A (Security)
1 sibling, 0 replies; 6+ messages in thread
From: Lopez, Jorge A (Security) @ 2023-08-11 15:42 UTC (permalink / raw)
To: Jiapeng Chong
Cc: hdegoede@redhat.com, markgross@kernel.org,
platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
Abaci Robot
Reviewed-by: Jorge Lopez <jorge.lopez2@hp.com>
Regards,
Jorge Lopez
HP Inc
"Once you stop learning, you start dying"
Albert Einstein
> -----Original Message-----
> From: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> Sent: Friday, August 11, 2023 1:30 AM
> To: Lopez, Jorge A (Security) <jorge.lopez2@hp.com>
> Cc: hdegoede@redhat.com; markgross@kernel.org; platform-driver-
> x86@vger.kernel.org; linux-kernel@vger.kernel.org; Jiapeng Chong
> <jiapeng.chong@linux.alibaba.com>; Abaci Robot
> <abaci@linux.alibaba.com>
> Subject: [PATCH] platform/x86: hp-bioscfg: Remove useless else
>
> CAUTION: External Email
>
> 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:545:3-5:
> WARNING: possible condition with no effect (if == else).
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.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 03d0188804ba..771e554182dc 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -541,14 +541,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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] platform/x86: hp-bioscfg: Remove useless else
@ 2024-01-03 2:17 Jiapeng Chong
2024-01-03 11:53 ` Ilpo Järvinen
0 siblings, 1 reply; 6+ messages in thread
From: Jiapeng Chong @ 2024-01-03 2:17 UTC (permalink / raw)
To: jorge.lopez2
Cc: hdegoede, ilpo.jarvinen, platform-driver-x86, linux-kernel,
Jiapeng Chong, Abaci Robot
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@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7817
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.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);
--
2.20.1.7.g153144c
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] platform/x86: hp-bioscfg: Remove useless else
2024-01-03 2:17 Jiapeng Chong
@ 2024-01-03 11:53 ` Ilpo Järvinen
0 siblings, 0 replies; 6+ messages in thread
From: Ilpo Järvinen @ 2024-01-03 11:53 UTC (permalink / raw)
To: Jiapeng Chong
Cc: jorge.lopez2, Hans de Goede, platform-driver-x86, LKML,
Abaci Robot
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@linux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7817
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-03 11:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-11 6:29 [PATCH] platform/x86: hp-bioscfg: Remove useless else Jiapeng Chong
2023-08-11 8:04 ` Ilpo Järvinen
2023-08-11 15:41 ` Lopez, Jorge A (Security)
2023-08-11 15:42 ` Lopez, Jorge A (Security)
-- strict thread matches above, loose matches on Subject: below --
2024-01-03 2:17 Jiapeng Chong
2024-01-03 11:53 ` Ilpo Järvinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox