* [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_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: 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: 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, 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 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 (?) (?) @ 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.