All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe()
@ 2018-01-31 21:26 ` SF Markus Elfring
  0 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-01-31 21:26 UTC (permalink / raw)
  To: linux-iio, linux-input, Hartmut Knaack, Jiri Kosina,
	Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Srinivas Pandruvada
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 31 Jan 2018 22:20:56 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/iio/accel/hid-sensor-accel-3d.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index c066a3bdbff7..3d0acde40285 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -383,11 +383,9 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 		return ret;
 	}
 	indio_dev->channels = kmemdup(channel_spec, channel_size, GFP_KERNEL);
-
-	if (!indio_dev->channels) {
-		dev_err(&pdev->dev, "failed to duplicate channels\n");
+	if (!indio_dev->channels)
 		return -ENOMEM;
-	}
+
 	ret = accel_3d_parse_report(pdev, hsdev,
 				(struct iio_chan_spec *)indio_dev->channels,
 				hsdev->usage, accel_state);
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_
@ 2018-01-31 21:26 ` SF Markus Elfring
  0 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-01-31 21:26 UTC (permalink / raw)
  To: linux-iio, linux-input, Hartmut Knaack, Jiri Kosina,
	Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Srinivas Pandruvada
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 31 Jan 2018 22:20:56 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/iio/accel/hid-sensor-accel-3d.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index c066a3bdbff7..3d0acde40285 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -383,11 +383,9 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 		return ret;
 	}
 	indio_dev->channels = kmemdup(channel_spec, channel_size, GFP_KERNEL);
-
-	if (!indio_dev->channels) {
-		dev_err(&pdev->dev, "failed to duplicate channels\n");
+	if (!indio_dev->channels)
 		return -ENOMEM;
-	}
+
 	ret = accel_3d_parse_report(pdev, hsdev,
 				(struct iio_chan_spec *)indio_dev->channels,
 				hsdev->usage, accel_state);
-- 
2.16.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe()
  2018-01-31 21:26 ` [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_ SF Markus Elfring
@ 2018-02-04 11:23   ` Jonathan Cameron
  -1 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-04 11:23 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-iio, linux-input, Hartmut Knaack, Jiri Kosina,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Srinivas Pandruvada,
	LKML, kernel-janitors

On Wed, 31 Jan 2018 22:26:14 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 31 Jan 2018 22:20:56 +0100
> 
> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Marcus,

If making changes like this I would suggest only sending one until
you have have a response from the relevant maintainer.

It would save you time as often these sorts of changes are
a matter of personal taste and weighing up of costs vs gains
- hence it is not obvious that they will be accepted.

Jonathan

> ---
>  drivers/iio/accel/hid-sensor-accel-3d.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index c066a3bdbff7..3d0acde40285 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
> @@ -383,11 +383,9 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  	indio_dev->channels = kmemdup(channel_spec, channel_size, GFP_KERNEL);
> -
> -	if (!indio_dev->channels) {
> -		dev_err(&pdev->dev, "failed to duplicate channels\n");
> +	if (!indio_dev->channels)
>  		return -ENOMEM;
> -	}
> +
>  	ret = accel_3d_parse_report(pdev, hsdev,
>  				(struct iio_chan_spec *)indio_dev->channels,
>  				hsdev->usage, accel_state);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel
@ 2018-02-04 11:23   ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-04 11:23 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-iio, linux-input, Hartmut Knaack, Jiri Kosina,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Srinivas Pandruvada,
	LKML, kernel-janitors

On Wed, 31 Jan 2018 22:26:14 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 31 Jan 2018 22:20:56 +0100
> 
> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Marcus,

If making changes like this I would suggest only sending one until
you have have a response from the relevant maintainer.

It would save you time as often these sorts of changes are
a matter of personal taste and weighing up of costs vs gains
- hence it is not obvious that they will be accepted.

Jonathan

> ---
>  drivers/iio/accel/hid-sensor-accel-3d.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index c066a3bdbff7..3d0acde40285 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
> @@ -383,11 +383,9 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  	indio_dev->channels = kmemdup(channel_spec, channel_size, GFP_KERNEL);
> -
> -	if (!indio_dev->channels) {
> -		dev_err(&pdev->dev, "failed to duplicate channels\n");
> +	if (!indio_dev->channels)
>  		return -ENOMEM;
> -	}
> +
>  	ret = accel_3d_parse_report(pdev, hsdev,
>  				(struct iio_chan_spec *)indio_dev->channels,
>  				hsdev->usage, accel_state);


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe()
  2018-02-04 11:23   ` [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel Jonathan Cameron
@ 2018-02-05 18:26     ` SF Markus Elfring
  -1 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-02-05 18:26 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, linux-input
  Cc: Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors

> If making changes like this I would suggest only sending one until
> you have have a response from the relevant maintainer.

The corresponding feedback can become more positive for such
a transformation pattern after a while, can't it?


> It would save you time as often these sorts of changes are
> a matter of personal taste and weighing up of costs vs gains
> - hence it is not obvious that they will be accepted.

Can the wording “WARNING: Possible unnecessary 'out of memory' message”
(from the script “checkpatch.pl”) be another incentive?

Regards,
Markus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_prob
@ 2018-02-05 18:26     ` SF Markus Elfring
  0 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-02-05 18:26 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, linux-input
  Cc: Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors

> If making changes like this I would suggest only sending one until
> you have have a response from the relevant maintainer.

The corresponding feedback can become more positive for such
a transformation pattern after a while, can't it?


> It would save you time as often these sorts of changes are
> a matter of personal taste and weighing up of costs vs gains
> - hence it is not obvious that they will be accepted.

Can the wording “WARNING: Possible unnecessary 'out of memory' message”
(from the script “checkpatch.pl”) be another incentive?

Regards,
Markus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe()
  2018-02-05 18:26     ` hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_prob SF Markus Elfring
  (?)
@ 2018-02-05 21:51       ` Jonathan Cameron
  -1 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-05 21:51 UTC (permalink / raw)
  To: SF Markus Elfring, Jonathan Cameron, linux-iio, linux-input
  Cc: Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors



On 5 February 2018 18:26:59 GMT, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
>> If making changes like this I would suggest only sending one until
>> you have have a response from the relevant maintainer.
>
>The corresponding feedback can become more positive for such
>a transformation pattern after a while, can't it?

Not in that sort of time period.
>
>
>> It would save you time as often these sorts of changes are
>> a matter of personal taste and weighing up of costs vs gains
>> - hence it is not obvious that they will be accepted.
>
>Can the wording “WARNING: Possible unnecessary 'out of memory' message”
>(from the script “checkpatch.pl”) be another incentive?

No. The key word is possible. Check patch is dumb and often gives false positives.
>
>Regards,
>Markus

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_prob
@ 2018-02-05 21:51       ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-05 21:51 UTC (permalink / raw)
  To: SF Markus Elfring, Jonathan Cameron, linux-iio, linux-input
  Cc: Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors



On 5 February 2018 18:26:59 GMT, SF Markus Elfring <elfring@users.sourceforge.net> wrote:
>> If making changes like this I would suggest only sending one until
>> you have have a response from the relevant maintainer.
>
>The corresponding feedback can become more positive for such
>a transformation pattern after a while, can't it?

Not in that sort of time period.
>
>
>> It would save you time as often these sorts of changes are
>> a matter of personal taste and weighing up of costs vs gains
>> - hence it is not obvious that they will be accepted.
>
>Can the wording “WARNING: Possible unnecessary 'out of memory' message”
>(from the script “checkpatch.pl”) be another incentive?

No. The key word is possible. Check patch is dumb and often gives false positives.
>
>Regards,
>Markus

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe()
@ 2018-02-05 21:51       ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-05 21:51 UTC (permalink / raw)
  To: SF Markus Elfring, Jonathan Cameron,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA
  Cc: Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA



On 5 February 2018 18:26:59 GMT, SF Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org> wrote:
>> If making changes like this I would suggest only sending one until
>> you have have a response from the relevant maintainer.
>
>The corresponding feedback can become more positive for such
>a transformation pattern after a while, can't it?

Not in that sort of time period.
>
>
>> It would save you time as often these sorts of changes are
>> a matter of personal taste and weighing up of costs vs gains
>> - hence it is not obvious that they will be accepted.
>
>Can the wording “WARNING: Possible unnecessary 'out of memory' message”
>(from the script “checkpatch.pl”) be another incentive?

No. The key word is possible. Check patch is dumb and often gives false positives.
>
>Regards,
>Markus

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
  2018-02-05 21:51       ` hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_prob Jonathan Cameron
@ 2018-02-06  8:45         ` SF Markus Elfring
  -1 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-02-06  8:45 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, linux-input
  Cc: Jonathan Cameron, Hartmut Knaack, Jiri Kosina,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Srinivas Pandruvada,
	LKML, kernel-janitors

> Check patch is dumb and often gives false positives.

Would you like to improve this software situation anyhow?

Regards,
Markus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-06  8:45         ` SF Markus Elfring
  0 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-02-06  8:45 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, linux-input
  Cc: Jonathan Cameron, Hartmut Knaack, Jiri Kosina,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Srinivas Pandruvada,
	LKML, kernel-janitors

> Check patch is dumb and often gives false positives.

Would you like to improve this software situation anyhow?

Regards,
Markus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
  2018-02-06  8:45         ` SF Markus Elfring
  (?)
@ 2018-02-10 14:53         ` Jonathan Cameron
  2018-02-10 14:59             ` Joe Perches
  -1 siblings, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-10 14:53 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-iio, linux-input, Jonathan Cameron, Hartmut Knaack,
	Jiri Kosina, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Srinivas Pandruvada, LKML, kernel-janitors

On Tue, 6 Feb 2018 09:45:48 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> > Check patch is dumb and often gives false positives.  
> 
> Would you like to improve this software situation anyhow?

While it would be great to improve checkpatches false
positive rate, it's very nature as a string matcher makes
this hard.

Jonathan

> 
> Regards,
> Markus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
  2018-02-10 14:53         ` Jonathan Cameron
  2018-02-10 14:59             ` Joe Perches
  (?)
@ 2018-02-10 14:59             ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 14:59 UTC (permalink / raw)
  To: Jonathan Cameron, SF Markus Elfring
  Cc: linux-iio, linux-input, Jonathan Cameron, Hartmut Knaack,
	Jiri Kosina, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Srinivas Pandruvada, LKML, kernel-janitors

On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> While it would be great to improve checkpatches false
> positive rate, it's very nature as a string matcher makes
> this hard.

true.

what are the false positives you see?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 14:59             ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 14:59 UTC (permalink / raw)
  To: Jonathan Cameron, SF Markus Elfring
  Cc: linux-iio, linux-input, Jonathan Cameron, Hartmut Knaack,
	Jiri Kosina, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Srinivas Pandruvada, LKML, kernel-janitors

On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> While it would be great to improve checkpatches false
> positive rate, it's very nature as a string matcher makes
> this hard.

true.

what are the false positives you see?


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 14:59             ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 14:59 UTC (permalink / raw)
  To: Jonathan Cameron, SF Markus Elfring
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> While it would be great to improve checkpatches false
> positive rate, it's very nature as a string matcher makes
> this hard.

true.

what are the false positives you see?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 14:59             ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 14:59 UTC (permalink / raw)
  To: Jonathan Cameron, SF Markus Elfring
  Cc: linux-iio, linux-input, Jonathan Cameron, Hartmut Knaack,
	Jiri Kosina, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Srinivas Pandruvada, LKML, kernel-janitors

On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> While it would be great to improve checkpatches false
> positive rate, it's very nature as a string matcher makes
> this hard.

true.

what are the false positives you see?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 15:57               ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-10 15:57 UTC (permalink / raw)
  To: Joe Perches
  Cc: SF Markus Elfring, linux-iio, linux-input, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors

On Sat, 10 Feb 2018 06:59:43 -0800
Joe Perches <joe@perches.com> wrote:

> On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > While it would be great to improve checkpatches false
> > positive rate, it's very nature as a string matcher makes
> > this hard.  
> 
> true.
> 
> what are the false positives you see?
> 
This particular case is only 'sort of' a false positive
in the warning that a message printed on a memory allocation
failure 'may' not add any information over the generic case.

Very hard to judge on whether it is useful to know more than
an allocation failed somewhere or not.

Message makes this clear:
>“WARNING: Possible unnecessary 'out of memory' message”
>(from the script “checkpatch.pl”)  

We also have the balance between any changes to existing code
adding 'some' maintenance overhead vs changing this stuff
in a new driver - which is what checkpatch is really intended
for.

So I think checkpatch is striking the right balance here in
how it warns.  Obviously if it could assess the text
and come to an informed decision that would be great but
we are some way from that ;)

Jonathan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 15:57               ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-10 15:57 UTC (permalink / raw)
  To: Joe Perches
  Cc: SF Markus Elfring, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

On Sat, 10 Feb 2018 06:59:43 -0800
Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:

> On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > While it would be great to improve checkpatches false
> > positive rate, it's very nature as a string matcher makes
> > this hard.  
> 
> true.
> 
> what are the false positives you see?
> 
This particular case is only 'sort of' a false positive
in the warning that a message printed on a memory allocation
failure 'may' not add any information over the generic case.

Very hard to judge on whether it is useful to know more than
an allocation failed somewhere or not.

Message makes this clear:
>“WARNING: Possible unnecessary 'out of memory' message”
>(from the script “checkpatch.pl”)  

We also have the balance between any changes to existing code
adding 'some' maintenance overhead vs changing this stuff
in a new driver - which is what checkpatch is really intended
for.

So I think checkpatch is striking the right balance here in
how it warns.  Obviously if it could assess the text
and come to an informed decision that would be great but
we are some way from that ;)

Jonathan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 15:57               ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2018-02-10 15:57 UTC (permalink / raw)
  To: Joe Perches
  Cc: SF Markus Elfring, linux-iio, linux-input, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors

On Sat, 10 Feb 2018 06:59:43 -0800
Joe Perches <joe@perches.com> wrote:

> On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > While it would be great to improve checkpatches false
> > positive rate, it's very nature as a string matcher makes
> > this hard. =20
>=20
> true.
>=20
> what are the false positives you see?
>=20
This particular case is only 'sort of' a false positive
in the warning that a message printed on a memory allocation
failure 'may' not add any information over the generic case.

Very hard to judge on whether it is useful to know more than
an allocation failed somewhere or not.

Message makes this clear:
>=E2=80=9CWARNING: Possible unnecessary 'out of memory' message=E2=80=9D
>(from the script =E2=80=9Ccheckpatch.pl=E2=80=9D) =20

We also have the balance between any changes to existing code
adding 'some' maintenance overhead vs changing this stuff
in a new driver - which is what checkpatch is really intended
for.

So I think checkpatch is striking the right balance here in
how it warns.  Obviously if it could assess the text
and come to an informed decision that would be great but
we are some way from that ;)

Jonathan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
  2018-02-10 15:57               ` Jonathan Cameron
  (?)
  (?)
@ 2018-02-10 17:42                 ` Joe Perches
  -1 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 17:42 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: SF Markus Elfring, linux-iio, linux-input, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors

On Sat, 2018-02-10 at 15:57 +0000, Jonathan Cameron wrote:
> On Sat, 10 Feb 2018 06:59:43 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > > While it would be great to improve checkpatches false
> > > positive rate, it's very nature as a string matcher makes
> > > this hard.  
> > 
> > true.
> > 
> > what are the false positives you see?
> > 
> 
> This particular case is only 'sort of' a false positive
> in the warning that a message printed on a memory allocation
> failure 'may' not add any information over the generic case.

Right.  So it's not a 'false positive' at all.
Are there any actual 'false positives' you know of?

> Very hard to judge on whether it is useful to know more than
> an allocation failed somewhere or not.
> 
> Message makes this clear:
> > “WARNING: Possible unnecessary 'out of memory' message”
> > (from the script “checkpatch.pl”)  
> 
> We also have the balance between any changes to existing code
> adding 'some' maintenance overhead vs changing this stuff
> in a new driver - which is what checkpatch is really intended
> for.

There's almost zero maintenance overhead here.
The time it takes for the back and forth
replies is likely larger.

> So I think checkpatch is striking the right balance here in
> how it warns.  Obviously if it could assess the text
> and come to an informed decision that would be great but
> we are some way from that ;)

The 'informed' bit is difficult as it is mostly
a political problem.

This particular message really is unnecessary as
the generic dump_stack on any normal allocation
(ie: without __GFP_WARN) already emits location
specific information.

Removing these messages can help make the kernel
image smaller and thereby help make these OOM
messages a tiny bit less likely.

I just wish Markus would improve his consistently
terrible commit messages that just restate the
action being done and detail _why_ a particular
thing _should_ be done.

His acceptance rate would improve as many of these
back and forth replies for what trivialities he
posts as patches would be minimized.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 17:42                 ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 17:42 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: SF Markus Elfring, linux-iio, linux-input, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors

On Sat, 2018-02-10 at 15:57 +0000, Jonathan Cameron wrote:
> On Sat, 10 Feb 2018 06:59:43 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > > While it would be great to improve checkpatches false
> > > positive rate, it's very nature as a string matcher makes
> > > this hard.  
> > 
> > true.
> > 
> > what are the false positives you see?
> > 
> 
> This particular case is only 'sort of' a false positive
> in the warning that a message printed on a memory allocation
> failure 'may' not add any information over the generic case.

Right.  So it's not a 'false positive' at all.
Are there any actual 'false positives' you know of?

> Very hard to judge on whether it is useful to know more than
> an allocation failed somewhere or not.
> 
> Message makes this clear:
> > “WARNING: Possible unnecessary 'out of memory' message”
> > (from the script “checkpatch.pl”)  
> 
> We also have the balance between any changes to existing code
> adding 'some' maintenance overhead vs changing this stuff
> in a new driver - which is what checkpatch is really intended
> for.

There's almost zero maintenance overhead here.
The time it takes for the back and forth
replies is likely larger.

> So I think checkpatch is striking the right balance here in
> how it warns.  Obviously if it could assess the text
> and come to an informed decision that would be great but
> we are some way from that ;)

The 'informed' bit is difficult as it is mostly
a political problem.

This particular message really is unnecessary as
the generic dump_stack on any normal allocation
(ie: without __GFP_WARN) already emits location
specific information.

Removing these messages can help make the kernel
image smaller and thereby help make these OOM
messages a tiny bit less likely.

I just wish Markus would improve his consistently
terrible commit messages that just restate the
action being done and detail _why_ a particular
thing _should_ be done.

His acceptance rate would improve as many of these
back and forth replies for what trivialities he
posts as patches would be minimized.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 17:42                 ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 17:42 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: SF Markus Elfring, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

On Sat, 2018-02-10 at 15:57 +0000, Jonathan Cameron wrote:
> On Sat, 10 Feb 2018 06:59:43 -0800
> Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:
> 
> > On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > > While it would be great to improve checkpatches false
> > > positive rate, it's very nature as a string matcher makes
> > > this hard.  
> > 
> > true.
> > 
> > what are the false positives you see?
> > 
> 
> This particular case is only 'sort of' a false positive
> in the warning that a message printed on a memory allocation
> failure 'may' not add any information over the generic case.

Right.  So it's not a 'false positive' at all.
Are there any actual 'false positives' you know of?

> Very hard to judge on whether it is useful to know more than
> an allocation failed somewhere or not.
> 
> Message makes this clear:
> > “WARNING: Possible unnecessary 'out of memory' message”
> > (from the script “checkpatch.pl”)  
> 
> We also have the balance between any changes to existing code
> adding 'some' maintenance overhead vs changing this stuff
> in a new driver - which is what checkpatch is really intended
> for.

There's almost zero maintenance overhead here.
The time it takes for the back and forth
replies is likely larger.

> So I think checkpatch is striking the right balance here in
> how it warns.  Obviously if it could assess the text
> and come to an informed decision that would be great but
> we are some way from that ;)

The 'informed' bit is difficult as it is mostly
a political problem.

This particular message really is unnecessary as
the generic dump_stack on any normal allocation
(ie: without __GFP_WARN) already emits location
specific information.

Removing these messages can help make the kernel
image smaller and thereby help make these OOM
messages a tiny bit less likely.

I just wish Markus would improve his consistently
terrible commit messages that just restate the
action being done and detail _why_ a particular
thing _should_ be done.

His acceptance rate would improve as many of these
back and forth replies for what trivialities he
posts as patches would be minimized.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 17:42                 ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-02-10 17:42 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: SF Markus Elfring, linux-iio, linux-input, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors

On Sat, 2018-02-10 at 15:57 +0000, Jonathan Cameron wrote:
> On Sat, 10 Feb 2018 06:59:43 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > > While it would be great to improve checkpatches false
> > > positive rate, it's very nature as a string matcher makes
> > > this hard.  
> > 
> > true.
> > 
> > what are the false positives you see?
> > 
> 
> This particular case is only 'sort of' a false positive
> in the warning that a message printed on a memory allocation
> failure 'may' not add any information over the generic case.

Right.  So it's not a 'false positive' at all.
Are there any actual 'false positives' you know of?

> Very hard to judge on whether it is useful to know more than
> an allocation failed somewhere or not.
> 
> Message makes this clear:
> > “WARNING: Possible unnecessary 'out of memory' message”
> > (from the script “checkpatch.pl”)  
> 
> We also have the balance between any changes to existing code
> adding 'some' maintenance overhead vs changing this stuff
> in a new driver - which is what checkpatch is really intended
> for.

There's almost zero maintenance overhead here.
The time it takes for the back and forth
replies is likely larger.

> So I think checkpatch is striking the right balance here in
> how it warns.  Obviously if it could assess the text
> and come to an informed decision that would be great but
> we are some way from that ;)

The 'informed' bit is difficult as it is mostly
a political problem.

This particular message really is unnecessary as
the generic dump_stack on any normal allocation
(ie: without __GFP_WARN) already emits location
specific information.

Removing these messages can help make the kernel
image smaller and thereby help make these OOM
messages a tiny bit less likely.

I just wish Markus would improve his consistently
terrible commit messages that just restate the
action being done and detail _why_ a particular
thing _should_ be done.

His acceptance rate would improve as many of these
back and forth replies for what trivialities he
posts as patches would be minimized.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
  2018-02-10 17:42                 ` Joe Perches
@ 2018-02-10 18:30                   ` SF Markus Elfring
  -1 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-02-10 18:30 UTC (permalink / raw)
  To: Joe Perches, Jonathan Cameron, linux-iio, linux-input
  Cc: Jonathan Cameron, Hartmut Knaack, Jiri Kosina,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Srinivas Pandruvada,
	LKML, kernel-janitors

>> So I think checkpatch is striking the right balance here in
>> how it warns.  Obviously if it could assess the text
>> and come to an informed decision that would be great but
>> we are some way from that ;)
> 
> The 'informed' bit is difficult as it is mostly a political problem.

I find such a view very interesting.


> I just wish Markus would improve his consistently terrible commit messages

I tried to achieve another clarification a few times.


> that just restate the action being done and detail
> _why_ a particular thing _should_ be done.

Unfortunately, it seems that no other contributors picked
corresponding opportunities up so far.

You indicated also special software development challenges in your commit
“checkpatch: attempt to find unnecessary 'out of memory' messages”.

https://lkml.org/lkml/2014/6/10/382
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ebfdc40969f24fc0cdd1349835d36e8ebae05374


> His acceptance rate would improve as many of these back and forth
> replies for what trivialities he posts as patches would be minimized.

My selection of change possibilities leads to mixed integration results.

I stumbled on variations for general change resistance.

Regards,
Markus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Software evolution around “checkpatch.pl”?
@ 2018-02-10 18:30                   ` SF Markus Elfring
  0 siblings, 0 replies; 25+ messages in thread
From: SF Markus Elfring @ 2018-02-10 18:30 UTC (permalink / raw)
  To: Joe Perches, Jonathan Cameron, linux-iio, linux-input
  Cc: Jonathan Cameron, Hartmut Knaack, Jiri Kosina,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Srinivas Pandruvada,
	LKML, kernel-janitors

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1444 bytes --]

>> So I think checkpatch is striking the right balance here in
>> how it warns.  Obviously if it could assess the text
>> and come to an informed decision that would be great but
>> we are some way from that ;)
> 
> The 'informed' bit is difficult as it is mostly a political problem.

I find such a view very interesting.


> I just wish Markus would improve his consistently terrible commit messages

I tried to achieve another clarification a few times.


> that just restate the action being done and detail
> _why_ a particular thing _should_ be done.

Unfortunately, it seems that no other contributors picked
corresponding opportunities up so far.

You indicated also special software development challenges in your commit
“checkpatch: attempt to find unnecessary 'out of memory' messages”.

https://lkml.org/lkml/2014/6/10/382
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?idëfdc40969f24fc0cdd1349835d36e8ebae05374


> His acceptance rate would improve as many of these back and forth
> replies for what trivialities he posts as patches would be minimized.

My selection of change possibilities leads to mixed integration results.

I stumbled on variations for general change resistance.

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-02-10 18:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-31 21:26 [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe() SF Markus Elfring
2018-01-31 21:26 ` [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_ SF Markus Elfring
2018-02-04 11:23 ` [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe() Jonathan Cameron
2018-02-04 11:23   ` [PATCH] hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel Jonathan Cameron
2018-02-05 18:26   ` hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe() SF Markus Elfring
2018-02-05 18:26     ` hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_prob SF Markus Elfring
2018-02-05 21:51     ` hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe() Jonathan Cameron
2018-02-05 21:51       ` Jonathan Cameron
2018-02-05 21:51       ` hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_prob Jonathan Cameron
2018-02-06  8:45       ` Software evolution around “checkpatch.pl”? SF Markus Elfring
2018-02-06  8:45         ` SF Markus Elfring
2018-02-10 14:53         ` Jonathan Cameron
2018-02-10 14:59           ` Joe Perches
2018-02-10 14:59             ` Joe Perches
2018-02-10 14:59             ` Joe Perches
2018-02-10 14:59             ` Joe Perches
2018-02-10 15:57             ` Jonathan Cameron
2018-02-10 15:57               ` Jonathan Cameron
2018-02-10 15:57               ` Jonathan Cameron
2018-02-10 17:42               ` Joe Perches
2018-02-10 17:42                 ` Joe Perches
2018-02-10 17:42                 ` Joe Perches
2018-02-10 17:42                 ` Joe Perches
2018-02-10 18:30                 ` SF Markus Elfring
2018-02-10 18:30                   ` SF Markus Elfring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.