From: Markus Elfring <[email protected]>
Date: Mon, 24 Nov 2014 20:30:29 +0100
The backlight_device_unregister() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/platform/x86/asus-laptop.c | 3 +--
drivers/platform/x86/asus-wmi.c | 3 +--
drivers/platform/x86/eeepc-laptop.c | 3 +--
drivers/platform/x86/fujitsu-laptop.c | 6 ++----
drivers/platform/x86/ideapad-laptop.c | 3 +--
drivers/platform/x86/intel_oaktrail.c | 3 +--
drivers/platform/x86/msi-wmi.c | 3 +--
drivers/platform/x86/sony-laptop.c | 3 +--
drivers/platform/x86/toshiba_acpi.c | 3 +--
9 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 7f4dc6f..11fac1a 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -843,8 +843,7 @@ static int asus_backlight_init(struct asus_laptop *asus)
static void asus_backlight_exit(struct asus_laptop *asus)
{
- if (asus->backlight_device)
- backlight_device_unregister(asus->backlight_device);
+ backlight_device_unregister(asus->backlight_device);
asus->backlight_device = NULL;
}
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 21fc932..7543a56 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1308,8 +1308,7 @@ static int asus_wmi_backlight_init(struct asus_wmi *asus)
static void asus_wmi_backlight_exit(struct asus_wmi *asus)
{
- if (asus->backlight_device)
- backlight_device_unregister(asus->backlight_device);
+ backlight_device_unregister(asus->backlight_device);
asus->backlight_device = NULL;
}
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index bd533c2..9bc7eb7 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1158,8 +1158,7 @@ static int eeepc_backlight_init(struct eeepc_laptop *eeepc)
static void eeepc_backlight_exit(struct eeepc_laptop *eeepc)
{
- if (eeepc->backlight_device)
- backlight_device_unregister(eeepc->backlight_device);
+ backlight_device_unregister(eeepc->backlight_device);
eeepc->backlight_device = NULL;
}
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 87aa28c..f1a670b 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -1147,8 +1147,7 @@ fail_hotkey1:
fail_hotkey:
platform_driver_unregister(&fujitsupf_driver);
fail_backlight:
- if (fujitsu->bl_device)
- backlight_device_unregister(fujitsu->bl_device);
+ backlight_device_unregister(fujitsu->bl_device);
fail_sysfs_group:
sysfs_remove_group(&fujitsu->pf_device->dev.kobj,
&fujitsupf_attribute_group);
@@ -1172,8 +1171,7 @@ static void __exit fujitsu_cleanup(void)
platform_driver_unregister(&fujitsupf_driver);
- if (fujitsu->bl_device)
- backlight_device_unregister(fujitsu->bl_device);
+ backlight_device_unregister(fujitsu->bl_device);
sysfs_remove_group(&fujitsu->pf_device->dev.kobj,
&fujitsupf_attribute_group);
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index ed494f3..3163061 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -729,8 +729,7 @@ static int ideapad_backlight_init(struct ideapad_private *priv)
static void ideapad_backlight_exit(struct ideapad_private *priv)
{
- if (priv->blightdev)
- backlight_device_unregister(priv->blightdev);
+ backlight_device_unregister(priv->blightdev);
priv->blightdev = NULL;
}
diff --git a/drivers/platform/x86/intel_oaktrail.c b/drivers/platform/x86/intel_oaktrail.c
index 4bc9604..0dd72cf 100644
--- a/drivers/platform/x86/intel_oaktrail.c
+++ b/drivers/platform/x86/intel_oaktrail.c
@@ -271,8 +271,7 @@ static int oaktrail_backlight_init(void)
static void oaktrail_backlight_exit(void)
{
- if (oaktrail_bl_device)
- backlight_device_unregister(oaktrail_bl_device);
+ backlight_device_unregister(oaktrail_bl_device);
}
static int oaktrail_probe(struct platform_device *pdev)
diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
index 70222f2..6d2bac0 100644
--- a/drivers/platform/x86/msi-wmi.c
+++ b/drivers/platform/x86/msi-wmi.c
@@ -354,8 +354,7 @@ static void __exit msi_wmi_exit(void)
sparse_keymap_free(msi_wmi_input_dev);
input_unregister_device(msi_wmi_input_dev);
}
- if (backlight)
- backlight_device_unregister(backlight);
+ backlight_device_unregister(backlight);
}
module_init(msi_wmi_init);
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 26ad9ff..a48cdc7 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -3141,8 +3141,7 @@ static void sony_nc_backlight_setup(void)
static void sony_nc_backlight_cleanup(void)
{
- if (sony_bl_props.dev)
- backlight_device_unregister(sony_bl_props.dev);
+ backlight_device_unregister(sony_bl_props.dev);
}
static int sony_nc_add(struct acpi_device *device)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index d0dce73..703a68a 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1647,8 +1647,7 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
rfkill_destroy(dev->bt_rfk);
}
- if (dev->backlight_dev)
- backlight_device_unregister(dev->backlight_dev);
+ backlight_device_unregister(dev->backlight_dev);
if (dev->illumination_supported)
led_classdev_unregister(&dev->led_dev);
--
2.1.3
Hi Markus,
Le lundi 24 novembre 2014, 20:40:22 SF Markus Elfring a ?crit :
> From: Markus Elfring <[email protected]>
> Date: Mon, 24 Nov 2014 20:30:29 +0100
>
> The backlight_device_unregister() function tests whether its argument is
> NULL and then returns immediately. Thus the test around the call is not
> needed.
>
> This issue was detected by using the Coccinelle software.
What script was used ? Is it in scripts/coccinelle ?
> diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
> index 70222f2..6d2bac0 100644
> --- a/drivers/platform/x86/msi-wmi.c
> +++ b/drivers/platform/x86/msi-wmi.c
> @@ -354,8 +354,7 @@ static void __exit msi_wmi_exit(void)
> sparse_keymap_free(msi_wmi_input_dev);
> input_unregister_device(msi_wmi_input_dev);
> }
> - if (backlight)
> - backlight_device_unregister(backlight);
> + backlight_device_unregister(backlight);
> }
>
> module_init(msi_wmi_init);
For this part:
Acked-by: Anisse Astier <[email protected]>
Regards,
Anisse
>> This issue was detected by using the Coccinelle software.
>
> What script was used ?
A semantic patch approach which I published on the mailing lists in March
is in action on my software development system for a while.
> Is it in scripts/coccinelle ?
Not yet.
I hope that the involved update suggestions got sufficient positive feedback
to integrate five scripts there.
Regards,
Markus
On Mon, Nov 24, 2014 at 08:40:22PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Mon, 24 Nov 2014 20:30:29 +0100
>
> The backlight_device_unregister() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <[email protected]>
Thanks Markus, queued.
--
Darren Hart
Intel Open Source Technology Center
On Mon, 24 Nov 2014, SF Markus Elfring wrote:
> >> This issue was detected by using the Coccinelle software.
> >
> > What script was used ?
>
> A semantic patch approach which I published on the mailing lists in March
> is in action on my software development system for a while.
>
>
> > Is it in scripts/coccinelle ?
>
> Not yet.
>
> I hope that the involved update suggestions got sufficient positive feedback
> to integrate five scripts there.
The current scripts are very complicated, involving the interaction
between multiple scripts and a database, and I think they are not very
suitable for make coccicheck. Also, the idea of removing the null checks
is not appropriate in all contexts. Perhaps it could be possible to add
a script to the Linux kernel that handles a number of common cases for
which removing the null test is widely considered to be desirable.
julia
On Thu, Nov 27, 2014 at 07:13:10PM +0100, Julia Lawall wrote:
>
>
> On Mon, 24 Nov 2014, SF Markus Elfring wrote:
>
> > >> This issue was detected by using the Coccinelle software.
> > >
> > > What script was used ?
> >
> > A semantic patch approach which I published on the mailing lists in March
> > is in action on my software development system for a while.
> >
> >
> > > Is it in scripts/coccinelle ?
> >
> > Not yet.
> >
> > I hope that the involved update suggestions got sufficient positive feedback
> > to integrate five scripts there.
>
> The current scripts are very complicated, involving the interaction
> between multiple scripts and a database, and I think they are not very
> suitable for make coccicheck. Also, the idea of removing the null checks
> is not appropriate in all contexts. Perhaps it could be possible to add
> a script to the Linux kernel that handles a number of common cases for
> which removing the null test is widely considered to be desirable.
>
> julia
>
Julia, do you have any particular objection to this specific patch? I didn't see
a reason to prevent it going in.
--
Darren Hart
Intel Open Source Technology Center
On Fri, 26 Jun 2015, Darren Hart wrote:
> On Thu, Nov 27, 2014 at 07:13:10PM +0100, Julia Lawall wrote:
> >
> >
> > On Mon, 24 Nov 2014, SF Markus Elfring wrote:
> >
> > > >> This issue was detected by using the Coccinelle software.
> > > >
> > > > What script was used ?
> > >
> > > A semantic patch approach which I published on the mailing lists in March
> > > is in action on my software development system for a while.
> > >
> > >
> > > > Is it in scripts/coccinelle ?
> > >
> > > Not yet.
> > >
> > > I hope that the involved update suggestions got sufficient positive feedback
> > > to integrate five scripts there.
> >
> > The current scripts are very complicated, involving the interaction
> > between multiple scripts and a database, and I think they are not very
> > suitable for make coccicheck. Also, the idea of removing the null checks
> > is not appropriate in all contexts. Perhaps it could be possible to add
> > a script to the Linux kernel that handles a number of common cases for
> > which removing the null test is widely considered to be desirable.
> >
> > julia
> >
>
> Julia, do you have any particular objection to this specific patch? I didn't see
> a reason to prevent it going in.
Sorry if I was unclear. If there is no problem with the current patch, I
have no objection to it. I don't think that the semantic patch that
caused this patch to be generated is suitable for inclusion in the Linux
kernel.
julia
> Julia, do you have any particular objection to this specific patch?
> I didn't see a reason to prevent it going in.
Thanks for your interest around this concrete update suggestion.
* Would you like to distinguish the consequences a bit more
for results from the application of the semantic patch language?
* Where do you prefer to store special extensions for
Coccinelle's script collection
Regards,
Markus
On Fri, Jun 26, 2015 at 04:06:55PM -0700, Darren Hart wrote:
> Julia, do you have any particular objection to this specific patch? I didn't see
> a reason to prevent it going in.
I hate these patches...
We're saying "these functions have sanity checks so let's pass nonsense
values to them, it's fine." It makes the code harder to understand.
There is no way for a human being to remember the complete list of
functions with sanity checks and which don't have sanity checks.
Markus has introduced quite a few bugs as well (people have so far
managed to catch his bugs before they were committed). He so far has
resisted any suggestion that he should manually review his patches
before sending them.
Btw do have a scripts/coccinelle/free/ifnullfree.cocci which removes
checks before kfree, debugfs_remove, debugfs_remove_recursive, and
usb_free_urb.
regards,
dan carpenter
> There is no way for a human being to remember the complete list of
> functions with sanity checks and which don't have sanity checks.
I understand also this software development challenge.
> Markus has introduced quite a few bugs as well
I have only found other opinions about specific update suggestions.
Which of such "bugs" are real mistakes?
Are you looking for a better consensus?
> (people have so far managed to catch his bugs before they were committed).
Would you like to refer to any concrete software developers?
> He so far has resisted any suggestion that he should manually review
> his patches before sending them.
I am performing source code review to some degree.
My approach does partly not fit to your expectations.
Regards,
Markus