linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] coccinnelle: Remove ptr_ret script
@ 2020-01-07  7:36 Maxime Ripard
  2020-01-07  9:40 ` Jani Nikula
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Maxime Ripard @ 2020-01-07  7:36 UTC (permalink / raw)
  To: Julia.Lawall, Gilles.Muller, nicolas.palix, michal.lkml
  Cc: cocci, linux-kernel, Maxime Ripard, Jani Nikula, Thierry Reding,
	Tomi Valkeinen, Mark Brown

The ptr_ret script script addresses a number of situations where we end up
testing an error pointer, and if it's an error returning it, or return 0
otherwise to transform it into a PTR_ERR_OR_ZERO call.

So it will convert a block like this:

if (IS_ERR(err))
    return PTR_ERR(err);

return 0;

into

return PTR_ERR_OR_ZERO(err);

While this is technically correct, it has a number of drawbacks. First, it
merges the error and success path, which will make it harder for a reviewer
or reader to grasp.

It's also more difficult to extend if we were to add some code between the
error check and the function return, making the author essentially revert
that patch before adding new lines, while it would have been a trivial
addition otherwise for the rewiever.

Therefore, since that script is only about cosmetic in the first place,
let's remove it since it's not worth it.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 scripts/coccinelle/api/ptr_ret.cocci | 97 ----------------------------
 1 file changed, 97 deletions(-)
 delete mode 100644 scripts/coccinelle/api/ptr_ret.cocci

diff --git a/scripts/coccinelle/api/ptr_ret.cocci b/scripts/coccinelle/api/ptr_ret.cocci
deleted file mode 100644
index e76cd5d90a8a..000000000000
--- a/scripts/coccinelle/api/ptr_ret.cocci
+++ /dev/null
@@ -1,97 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-///
-/// Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
-///
-// Confidence: High
-// Copyright: (C) 2012 Julia Lawall, INRIA/LIP6.
-// Copyright: (C) 2012 Gilles Muller, INRIA/LiP6.
-// URL: http://coccinelle.lip6.fr/
-// Options: --no-includes --include-headers
-//
-// Keywords: ERR_PTR, PTR_ERR, PTR_ERR_OR_ZERO
-// Version min: 2.6.39
-//
-
-virtual context
-virtual patch
-virtual org
-virtual report
-
-@depends on patch@
-expression ptr;
-@@
-
-- if (IS_ERR(ptr)) return PTR_ERR(ptr); else return 0;
-+ return PTR_ERR_OR_ZERO(ptr);
-
-@depends on patch@
-expression ptr;
-@@
-
-- if (IS_ERR(ptr)) return PTR_ERR(ptr); return 0;
-+ return PTR_ERR_OR_ZERO(ptr);
-
-@depends on patch@
-expression ptr;
-@@
-
-- (IS_ERR(ptr) ? PTR_ERR(ptr) : 0)
-+ PTR_ERR_OR_ZERO(ptr)
-
-@r1 depends on !patch@
-expression ptr;
-position p1;
-@@
-
-* if@p1 (IS_ERR(ptr)) return PTR_ERR(ptr); else return 0;
-
-@r2 depends on !patch@
-expression ptr;
-position p2;
-@@
-
-* if@p2 (IS_ERR(ptr)) return PTR_ERR(ptr); return 0;
-
-@r3 depends on !patch@
-expression ptr;
-position p3;
-@@
-
-* IS_ERR@p3(ptr) ? PTR_ERR(ptr) : 0
-
-@script:python depends on org@
-p << r1.p1;
-@@
-
-coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
-
-
-@script:python depends on org@
-p << r2.p2;
-@@
-
-coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
-
-@script:python depends on org@
-p << r3.p3;
-@@
-
-coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
-
-@script:python depends on report@
-p << r1.p1;
-@@
-
-coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
-
-@script:python depends on report@
-p << r2.p2;
-@@
-
-coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
-
-@script:python depends on report@
-p << r3.p3;
-@@
-
-coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
-- 
2.24.1


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

* Re: [PATCH] coccinnelle: Remove ptr_ret script
  2020-01-07  7:36 [PATCH] coccinnelle: Remove ptr_ret script Maxime Ripard
@ 2020-01-07  9:40 ` Jani Nikula
  2020-01-07  9:40 ` Thierry Reding
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2020-01-07  9:40 UTC (permalink / raw)
  To: Maxime Ripard, Julia.Lawall, Gilles.Muller, nicolas.palix, michal.lkml
  Cc: cocci, linux-kernel, Maxime Ripard, Thierry Reding,
	Tomi Valkeinen, Mark Brown

On Tue, 07 Jan 2020, Maxime Ripard <maxime@cerno.tech> wrote:
> The ptr_ret script script addresses a number of situations where we end up
> testing an error pointer, and if it's an error returning it, or return 0
> otherwise to transform it into a PTR_ERR_OR_ZERO call.
>
> So it will convert a block like this:
>
> if (IS_ERR(err))
>     return PTR_ERR(err);
>
> return 0;
>
> into
>
> return PTR_ERR_OR_ZERO(err);
>
> While this is technically correct, it has a number of drawbacks. First, it
> merges the error and success path, which will make it harder for a reviewer
> or reader to grasp.
>
> It's also more difficult to extend if we were to add some code between the
> error check and the function return, making the author essentially revert
> that patch before adding new lines, while it would have been a trivial
> addition otherwise for the rewiever.
>
> Therefore, since that script is only about cosmetic in the first place,
> let's remove it since it's not worth it.

I guess Julia's patch [1] never got merged then?

Reaffirming my ack on the removal, regardless of whose patch it is,

Acked-by: Jani Nikula <jani.nikula@intel.com>


[1] http://lore.kernel.org/r/1574179017-23787-1-git-send-email-Julia.Lawall@lip6.fr


>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  scripts/coccinelle/api/ptr_ret.cocci | 97 ----------------------------
>  1 file changed, 97 deletions(-)
>  delete mode 100644 scripts/coccinelle/api/ptr_ret.cocci
>
> diff --git a/scripts/coccinelle/api/ptr_ret.cocci b/scripts/coccinelle/api/ptr_ret.cocci
> deleted file mode 100644
> index e76cd5d90a8a..000000000000
> --- a/scripts/coccinelle/api/ptr_ret.cocci
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -///
> -/// Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> -///
> -// Confidence: High
> -// Copyright: (C) 2012 Julia Lawall, INRIA/LIP6.
> -// Copyright: (C) 2012 Gilles Muller, INRIA/LiP6.
> -// URL: http://coccinelle.lip6.fr/
> -// Options: --no-includes --include-headers
> -//
> -// Keywords: ERR_PTR, PTR_ERR, PTR_ERR_OR_ZERO
> -// Version min: 2.6.39
> -//
> -
> -virtual context
> -virtual patch
> -virtual org
> -virtual report
> -
> -@depends on patch@
> -expression ptr;
> -@@
> -
> -- if (IS_ERR(ptr)) return PTR_ERR(ptr); else return 0;
> -+ return PTR_ERR_OR_ZERO(ptr);
> -
> -@depends on patch@
> -expression ptr;
> -@@
> -
> -- if (IS_ERR(ptr)) return PTR_ERR(ptr); return 0;
> -+ return PTR_ERR_OR_ZERO(ptr);
> -
> -@depends on patch@
> -expression ptr;
> -@@
> -
> -- (IS_ERR(ptr) ? PTR_ERR(ptr) : 0)
> -+ PTR_ERR_OR_ZERO(ptr)
> -
> -@r1 depends on !patch@
> -expression ptr;
> -position p1;
> -@@
> -
> -* if@p1 (IS_ERR(ptr)) return PTR_ERR(ptr); else return 0;
> -
> -@r2 depends on !patch@
> -expression ptr;
> -position p2;
> -@@
> -
> -* if@p2 (IS_ERR(ptr)) return PTR_ERR(ptr); return 0;
> -
> -@r3 depends on !patch@
> -expression ptr;
> -position p3;
> -@@
> -
> -* IS_ERR@p3(ptr) ? PTR_ERR(ptr) : 0
> -
> -@script:python depends on org@
> -p << r1.p1;
> -@@
> -
> -coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -
> -@script:python depends on org@
> -p << r2.p2;
> -@@
> -
> -coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on org@
> -p << r3.p3;
> -@@
> -
> -coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on report@
> -p << r1.p1;
> -@@
> -
> -coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on report@
> -p << r2.p2;
> -@@
> -
> -coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on report@
> -p << r3.p3;
> -@@
> -
> -coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH] coccinnelle: Remove ptr_ret script
  2020-01-07  7:36 [PATCH] coccinnelle: Remove ptr_ret script Maxime Ripard
  2020-01-07  9:40 ` Jani Nikula
@ 2020-01-07  9:40 ` Thierry Reding
  2020-01-07 10:06 ` Julia Lawall
  2020-01-07 11:48 ` Mark Brown
  3 siblings, 0 replies; 9+ messages in thread
From: Thierry Reding @ 2020-01-07  9:40 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Julia.Lawall, Gilles.Muller, nicolas.palix, michal.lkml, cocci,
	linux-kernel, Jani Nikula, Tomi Valkeinen, Mark Brown

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

On Tue, Jan 07, 2020 at 08:36:29AM +0100, Maxime Ripard wrote:
> The ptr_ret script script addresses a number of situations where we end up
> testing an error pointer, and if it's an error returning it, or return 0
> otherwise to transform it into a PTR_ERR_OR_ZERO call.
> 
> So it will convert a block like this:
> 
> if (IS_ERR(err))
>     return PTR_ERR(err);
> 
> return 0;
> 
> into
> 
> return PTR_ERR_OR_ZERO(err);
> 
> While this is technically correct, it has a number of drawbacks. First, it
> merges the error and success path, which will make it harder for a reviewer
> or reader to grasp.
> 
> It's also more difficult to extend if we were to add some code between the
> error check and the function return, making the author essentially revert
> that patch before adding new lines, while it would have been a trivial
> addition otherwise for the rewiever.
> 
> Therefore, since that script is only about cosmetic in the first place,
> let's remove it since it's not worth it.
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  scripts/coccinelle/api/ptr_ret.cocci | 97 ----------------------------
>  1 file changed, 97 deletions(-)
>  delete mode 100644 scripts/coccinelle/api/ptr_ret.cocci

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] coccinnelle: Remove ptr_ret script
  2020-01-07  7:36 [PATCH] coccinnelle: Remove ptr_ret script Maxime Ripard
  2020-01-07  9:40 ` Jani Nikula
  2020-01-07  9:40 ` Thierry Reding
@ 2020-01-07 10:06 ` Julia Lawall
  2020-01-07 10:29   ` [Cocci] " Wolfram Sang
  2020-01-07 11:48 ` Mark Brown
  3 siblings, 1 reply; 9+ messages in thread
From: Julia Lawall @ 2020-01-07 10:06 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Julia.Lawall, Gilles.Muller, nicolas.palix, michal.lkml, cocci,
	linux-kernel, Jani Nikula, Thierry Reding, Tomi Valkeinen,
	Mark Brown



On Tue, 7 Jan 2020, Maxime Ripard wrote:

> The ptr_ret script script addresses a number of situations where we end up
> testing an error pointer, and if it's an error returning it, or return 0
> otherwise to transform it into a PTR_ERR_OR_ZERO call.
>
> So it will convert a block like this:
>
> if (IS_ERR(err))
>     return PTR_ERR(err);
>
> return 0;
>
> into
>
> return PTR_ERR_OR_ZERO(err);
>
> While this is technically correct, it has a number of drawbacks. First, it
> merges the error and success path, which will make it harder for a reviewer
> or reader to grasp.
>
> It's also more difficult to extend if we were to add some code between the
> error check and the function return, making the author essentially revert
> that patch before adding new lines, while it would have been a trivial
> addition otherwise for the rewiever.
>
> Therefore, since that script is only about cosmetic in the first place,
> let's remove it since it's not worth it.
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Acked-by: Julia Lawall <julia.lawall@inria.fr>

> ---
>  scripts/coccinelle/api/ptr_ret.cocci | 97 ----------------------------
>  1 file changed, 97 deletions(-)
>  delete mode 100644 scripts/coccinelle/api/ptr_ret.cocci
>
> diff --git a/scripts/coccinelle/api/ptr_ret.cocci b/scripts/coccinelle/api/ptr_ret.cocci
> deleted file mode 100644
> index e76cd5d90a8a..000000000000
> --- a/scripts/coccinelle/api/ptr_ret.cocci
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -///
> -/// Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> -///
> -// Confidence: High
> -// Copyright: (C) 2012 Julia Lawall, INRIA/LIP6.
> -// Copyright: (C) 2012 Gilles Muller, INRIA/LiP6.
> -// URL: http://coccinelle.lip6.fr/
> -// Options: --no-includes --include-headers
> -//
> -// Keywords: ERR_PTR, PTR_ERR, PTR_ERR_OR_ZERO
> -// Version min: 2.6.39
> -//
> -
> -virtual context
> -virtual patch
> -virtual org
> -virtual report
> -
> -@depends on patch@
> -expression ptr;
> -@@
> -
> -- if (IS_ERR(ptr)) return PTR_ERR(ptr); else return 0;
> -+ return PTR_ERR_OR_ZERO(ptr);
> -
> -@depends on patch@
> -expression ptr;
> -@@
> -
> -- if (IS_ERR(ptr)) return PTR_ERR(ptr); return 0;
> -+ return PTR_ERR_OR_ZERO(ptr);
> -
> -@depends on patch@
> -expression ptr;
> -@@
> -
> -- (IS_ERR(ptr) ? PTR_ERR(ptr) : 0)
> -+ PTR_ERR_OR_ZERO(ptr)
> -
> -@r1 depends on !patch@
> -expression ptr;
> -position p1;
> -@@
> -
> -* if@p1 (IS_ERR(ptr)) return PTR_ERR(ptr); else return 0;
> -
> -@r2 depends on !patch@
> -expression ptr;
> -position p2;
> -@@
> -
> -* if@p2 (IS_ERR(ptr)) return PTR_ERR(ptr); return 0;
> -
> -@r3 depends on !patch@
> -expression ptr;
> -position p3;
> -@@
> -
> -* IS_ERR@p3(ptr) ? PTR_ERR(ptr) : 0
> -
> -@script:python depends on org@
> -p << r1.p1;
> -@@
> -
> -coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -
> -@script:python depends on org@
> -p << r2.p2;
> -@@
> -
> -coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on org@
> -p << r3.p3;
> -@@
> -
> -coccilib.org.print_todo(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on report@
> -p << r1.p1;
> -@@
> -
> -coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on report@
> -p << r2.p2;
> -@@
> -
> -coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> -
> -@script:python depends on report@
> -p << r3.p3;
> -@@
> -
> -coccilib.report.print_report(p[0], "WARNING: PTR_ERR_OR_ZERO can be used")
> --
> 2.24.1
>
>

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

* Re: [Cocci] [PATCH] coccinnelle: Remove ptr_ret script
  2020-01-07 10:06 ` Julia Lawall
@ 2020-01-07 10:29   ` Wolfram Sang
  2020-12-15  8:48     ` Maxime Ripard
  0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2020-01-07 10:29 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Maxime Ripard, michal.lkml, Gilles.Muller, Tomi Valkeinen,
	Mark Brown, nicolas.palix, linux-kernel, Jani Nikula,
	Julia.Lawall, Thierry Reding, cocci

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

On Tue, Jan 07, 2020 at 11:06:56AM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 7 Jan 2020, Maxime Ripard wrote:
> 
> > The ptr_ret script script addresses a number of situations where we end up
> > testing an error pointer, and if it's an error returning it, or return 0
> > otherwise to transform it into a PTR_ERR_OR_ZERO call.
> >
> > So it will convert a block like this:
> >
> > if (IS_ERR(err))
> >     return PTR_ERR(err);
> >
> > return 0;
> >
> > into
> >
> > return PTR_ERR_OR_ZERO(err);
> >
> > While this is technically correct, it has a number of drawbacks. First, it
> > merges the error and success path, which will make it harder for a reviewer
> > or reader to grasp.
> >
> > It's also more difficult to extend if we were to add some code between the
> > error check and the function return, making the author essentially revert
> > that patch before adding new lines, while it would have been a trivial
> > addition otherwise for the rewiever.
> >
> > Therefore, since that script is only about cosmetic in the first place,
> > let's remove it since it's not worth it.
> >
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> 
> Acked-by: Julia Lawall <julia.lawall@inria.fr>

Convincing patch description, good catch!

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] coccinnelle: Remove ptr_ret script
  2020-01-07  7:36 [PATCH] coccinnelle: Remove ptr_ret script Maxime Ripard
                   ` (2 preceding siblings ...)
  2020-01-07 10:06 ` Julia Lawall
@ 2020-01-07 11:48 ` Mark Brown
  3 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2020-01-07 11:48 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Julia.Lawall, Gilles.Muller, nicolas.palix, michal.lkml, cocci,
	linux-kernel, Jani Nikula, Thierry Reding, Tomi Valkeinen

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Tue, Jan 07, 2020 at 08:36:29AM +0100, Maxime Ripard wrote:
> The ptr_ret script script addresses a number of situations where we end up
> testing an error pointer, and if it's an error returning it, or return 0
> otherwise to transform it into a PTR_ERR_OR_ZERO call.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [Cocci] [PATCH] coccinnelle: Remove ptr_ret script
  2020-01-07 10:29   ` [Cocci] " Wolfram Sang
@ 2020-12-15  8:48     ` Maxime Ripard
  2020-12-15  8:52       ` Julia Lawall
  0 siblings, 1 reply; 9+ messages in thread
From: Maxime Ripard @ 2020-12-15  8:48 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Julia Lawall, michal.lkml, Gilles.Muller, Mark Brown,
	nicolas.palix, linux-kernel, Jani Nikula, Julia.Lawall,
	Tomi Valkeinen, Thierry Reding, cocci

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

Hi,

On Tue, Jan 07, 2020 at 11:29:54AM +0100, Wolfram Sang wrote:
> On Tue, Jan 07, 2020 at 11:06:56AM +0100, Julia Lawall wrote:
> > 
> > 
> > On Tue, 7 Jan 2020, Maxime Ripard wrote:
> > 
> > > The ptr_ret script script addresses a number of situations where we end up
> > > testing an error pointer, and if it's an error returning it, or return 0
> > > otherwise to transform it into a PTR_ERR_OR_ZERO call.
> > >
> > > So it will convert a block like this:
> > >
> > > if (IS_ERR(err))
> > >     return PTR_ERR(err);
> > >
> > > return 0;
> > >
> > > into
> > >
> > > return PTR_ERR_OR_ZERO(err);
> > >
> > > While this is technically correct, it has a number of drawbacks. First, it
> > > merges the error and success path, which will make it harder for a reviewer
> > > or reader to grasp.
> > >
> > > It's also more difficult to extend if we were to add some code between the
> > > error check and the function return, making the author essentially revert
> > > that patch before adding new lines, while it would have been a trivial
> > > addition otherwise for the rewiever.
> > >
> > > Therefore, since that script is only about cosmetic in the first place,
> > > let's remove it since it's not worth it.
> > >
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > Cc: Mark Brown <broonie@kernel.org>
> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > 
> > Acked-by: Julia Lawall <julia.lawall@inria.fr>
> 
> Convincing patch description, good catch!
> 
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

It looks like this patch was never applied, whose tree should it go
through?

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [Cocci] [PATCH] coccinnelle: Remove ptr_ret script
  2020-12-15  8:48     ` Maxime Ripard
@ 2020-12-15  8:52       ` Julia Lawall
  2020-12-15 12:25         ` Maxime Ripard
  0 siblings, 1 reply; 9+ messages in thread
From: Julia Lawall @ 2020-12-15  8:52 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Wolfram Sang, michal.lkml, Gilles.Muller, Mark Brown,
	nicolas.palix, linux-kernel, Jani Nikula, Julia Lawall,
	Tomi Valkeinen, Thierry Reding, cocci



On Tue, 15 Dec 2020, Maxime Ripard wrote:

> Hi,
>
> On Tue, Jan 07, 2020 at 11:29:54AM +0100, Wolfram Sang wrote:
> > On Tue, Jan 07, 2020 at 11:06:56AM +0100, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 7 Jan 2020, Maxime Ripard wrote:
> > >
> > > > The ptr_ret script script addresses a number of situations where we end up
> > > > testing an error pointer, and if it's an error returning it, or return 0
> > > > otherwise to transform it into a PTR_ERR_OR_ZERO call.
> > > >
> > > > So it will convert a block like this:
> > > >
> > > > if (IS_ERR(err))
> > > >     return PTR_ERR(err);
> > > >
> > > > return 0;
> > > >
> > > > into
> > > >
> > > > return PTR_ERR_OR_ZERO(err);
> > > >
> > > > While this is technically correct, it has a number of drawbacks. First, it
> > > > merges the error and success path, which will make it harder for a reviewer
> > > > or reader to grasp.
> > > >
> > > > It's also more difficult to extend if we were to add some code between the
> > > > error check and the function return, making the author essentially revert
> > > > that patch before adding new lines, while it would have been a trivial
> > > > addition otherwise for the rewiever.
> > > >
> > > > Therefore, since that script is only about cosmetic in the first place,
> > > > let's remove it since it's not worth it.
> > > >
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > > Cc: Mark Brown <broonie@kernel.org>
> > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > >
> > > Acked-by: Julia Lawall <julia.lawall@inria.fr>
> >
> > Convincing patch description, good catch!
> >
> > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> It looks like this patch was never applied, whose tree should it go
> through?

Sorry.  I can take it.  I'm not sure that I still have the original
message, though.  If you have it handy, that would be helpful.

julia

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

* Re: [Cocci] [PATCH] coccinnelle: Remove ptr_ret script
  2020-12-15  8:52       ` Julia Lawall
@ 2020-12-15 12:25         ` Maxime Ripard
  0 siblings, 0 replies; 9+ messages in thread
From: Maxime Ripard @ 2020-12-15 12:25 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Wolfram Sang, michal.lkml, Gilles.Muller, Mark Brown,
	nicolas.palix, linux-kernel, Jani Nikula, Julia Lawall,
	Tomi Valkeinen, Thierry Reding, cocci

[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]

On Tue, Dec 15, 2020 at 09:52:36AM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 15 Dec 2020, Maxime Ripard wrote:
> 
> > Hi,
> >
> > On Tue, Jan 07, 2020 at 11:29:54AM +0100, Wolfram Sang wrote:
> > > On Tue, Jan 07, 2020 at 11:06:56AM +0100, Julia Lawall wrote:
> > > >
> > > >
> > > > On Tue, 7 Jan 2020, Maxime Ripard wrote:
> > > >
> > > > > The ptr_ret script script addresses a number of situations where we end up
> > > > > testing an error pointer, and if it's an error returning it, or return 0
> > > > > otherwise to transform it into a PTR_ERR_OR_ZERO call.
> > > > >
> > > > > So it will convert a block like this:
> > > > >
> > > > > if (IS_ERR(err))
> > > > >     return PTR_ERR(err);
> > > > >
> > > > > return 0;
> > > > >
> > > > > into
> > > > >
> > > > > return PTR_ERR_OR_ZERO(err);
> > > > >
> > > > > While this is technically correct, it has a number of drawbacks. First, it
> > > > > merges the error and success path, which will make it harder for a reviewer
> > > > > or reader to grasp.
> > > > >
> > > > > It's also more difficult to extend if we were to add some code between the
> > > > > error check and the function return, making the author essentially revert
> > > > > that patch before adding new lines, while it would have been a trivial
> > > > > addition otherwise for the rewiever.
> > > > >
> > > > > Therefore, since that script is only about cosmetic in the first place,
> > > > > let's remove it since it's not worth it.
> > > > >
> > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > > > Cc: Mark Brown <broonie@kernel.org>
> > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > >
> > > > Acked-by: Julia Lawall <julia.lawall@inria.fr>
> > >
> > > Convincing patch description, good catch!
> > >
> > > Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >
> > It looks like this patch was never applied, whose tree should it go
> > through?
> 
> Sorry.  I can take it.  I'm not sure that I still have the original
> message, though.  If you have it handy, that would be helpful.

Sure, I just sent a new version with all the tags

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

end of thread, other threads:[~2020-12-15 12:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07  7:36 [PATCH] coccinnelle: Remove ptr_ret script Maxime Ripard
2020-01-07  9:40 ` Jani Nikula
2020-01-07  9:40 ` Thierry Reding
2020-01-07 10:06 ` Julia Lawall
2020-01-07 10:29   ` [Cocci] " Wolfram Sang
2020-12-15  8:48     ` Maxime Ripard
2020-12-15  8:52       ` Julia Lawall
2020-12-15 12:25         ` Maxime Ripard
2020-01-07 11:48 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).