Re: [PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2015-06-27 Thread Dan Carpenter 
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2015-06-27 Thread Julia Lawall 


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2015-06-26 Thread Darren Hart 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2014-11-27 Thread Julia Lawall 


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2014-11-24 Thread Darren Hart 
On Mon, Nov 24, 2014 at 08:40:22PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> 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 

Thanks Markus, queued.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2014-11-24 Thread SF Markus Elfring 
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2014-11-24 Thread Anisse Astier 
Hi Markus,

Le lundi 24 novembre 2014, 20:40:22 SF Markus Elfring a écrit :
> From: Markus Elfring 
> 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 

Regards,

Anisse
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/1] platform: x86: Deletion of checks before backlight_device_unregister()

2014-11-24 Thread SF Markus Elfring 
From: Markus Elfring 
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 
---
 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_d