* Re: [Cocci] [PATCH] coccinnelle: Remove ptr_ret script [not found] <20200107073629.325249-1-maxime@cerno.tech> @ 2020-01-07 10:06 ` Julia Lawall 2020-01-07 10:29 ` Wolfram Sang 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2020-01-07 10:06 UTC (permalink / raw) To: Maxime Ripard Cc: michal.lkml, Gilles.Muller, Tomi Valkeinen, Mark Brown, nicolas.palix, linux-kernel, Jani Nikula, Julia.Lawall, Thierry Reding, cocci 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 > > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Cocci] [PATCH] coccinnelle: Remove ptr_ret script 2020-01-07 10:06 ` [Cocci] [PATCH] coccinnelle: Remove ptr_ret script Julia Lawall @ 2020-01-07 10:29 ` Wolfram Sang 2020-12-15 8:48 ` Maxime Ripard 0 siblings, 1 reply; 5+ messages in thread From: Wolfram Sang @ 2020-01-07 10:29 UTC (permalink / raw) To: Julia Lawall Cc: michal.lkml, Gilles.Muller, Mark Brown, nicolas.palix, linux-kernel, Jani Nikula, Julia.Lawall, Tomi Valkeinen, Thierry Reding, Maxime Ripard, cocci [-- Attachment #1.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 #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Cocci] [PATCH] coccinnelle: Remove ptr_ret script 2020-01-07 10:29 ` Wolfram Sang @ 2020-12-15 8:48 ` Maxime Ripard 2020-12-15 8:52 ` Julia Lawall 0 siblings, 1 reply; 5+ messages in thread From: Maxime Ripard @ 2020-12-15 8:48 UTC (permalink / raw) To: Wolfram Sang Cc: michal.lkml, Gilles.Muller, Tomi Valkeinen, nicolas.palix, linux-kernel, Jani Nikula, Julia.Lawall, Mark Brown, Thierry Reding, cocci [-- Attachment #1.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 #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 5+ 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; 5+ messages in thread From: Julia Lawall @ 2020-12-15 8:52 UTC (permalink / raw) To: Maxime Ripard Cc: michal.lkml, Gilles.Muller, Tomi Valkeinen, nicolas.palix, linux-kernel, Jani Nikula, Julia Lawall, Mark Brown, 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 _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 5+ 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; 5+ messages in thread From: Maxime Ripard @ 2020-12-15 12:25 UTC (permalink / raw) To: Julia Lawall Cc: michal.lkml, Gilles.Muller, Tomi Valkeinen, nicolas.palix, linux-kernel, Jani Nikula, Julia Lawall, Mark Brown, Thierry Reding, cocci [-- Attachment #1.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 #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 136 bytes --] _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-15 12:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200107073629.325249-1-maxime@cerno.tech>
2020-01-07 10:06 ` [Cocci] [PATCH] coccinnelle: Remove ptr_ret script Julia Lawall
2020-01-07 10:29 ` Wolfram Sang
2020-12-15 8:48 ` Maxime Ripard
2020-12-15 8:52 ` Julia Lawall
2020-12-15 12:25 ` Maxime Ripard
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).