linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] coccinelle: misc: remove "complex return code" warnings
@ 2015-09-30 22:37 Johan Hovold
  2015-10-01  5:20 ` Julia Lawall
  2015-10-03 16:25 ` Julia Lawall
  0 siblings, 2 replies; 11+ messages in thread
From: Johan Hovold @ 2015-09-30 22:37 UTC (permalink / raw)
  To: Julia Lawall, Michal Marek
  Cc: Gilles Muller, Nicolas Palix, linux-kernel, cocci, Johan Hovold

This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
to delete overly complex return code processing").

There can be both symmetry and readability reasons for not wanting to do
the final function call as part of the return statement and to maintain
a clear separation of success and error paths.

Since this is in no way mandated by the coding standard, let's just
remove this semantic patch to avoid having "clean up" patches being
posted over and over in response to these Coccinelle warnings.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 scripts/coccinelle/misc/simple_return.cocci | 180 ----------------------------
 1 file changed, 180 deletions(-)
 delete mode 100644 scripts/coccinelle/misc/simple_return.cocci

diff --git a/scripts/coccinelle/misc/simple_return.cocci b/scripts/coccinelle/misc/simple_return.cocci
deleted file mode 100644
index e8b6313b116f..000000000000
--- a/scripts/coccinelle/misc/simple_return.cocci
+++ /dev/null
@@ -1,180 +0,0 @@
-/// Simplify a trivial if-return sequence.  Possibly combine with a
-/// preceding function call.
-///
-// Confidence: High
-// Copyright: (C) 2014 Julia Lawall, INRIA/LIP6.  GPLv2.
-// Copyright: (C) 2014 Gilles Muller, INRIA/LiP6.  GPLv2.
-// URL: http://coccinelle.lip6.fr/
-// Comments:
-// Options: --no-includes --include-headers
-
-virtual patch
-virtual context
-virtual org
-virtual report
-
-@r depends on patch@
-local idexpression e;
-identifier i,f,fn;
-@@
-
-fn(...) { <...
-- e@i =
-+ return
-    f(...);
--if (i != 0) return i;
--return 0;
-...> }
-
-@depends on patch@
-identifier r.i;
-type t;
-@@
-
--t i;
- ... when != i
-
-@depends on patch@
-expression e;
-@@
-
--if (e != 0)
-   return e;
--return 0;
-
-// -----------------------------------------------------------------------
-
-@s1 depends on context || org || report@
-local idexpression e;
-identifier i,f,fn;
-position p,p1,p2;
-@@
-
-fn(...) { <...
-* e@i@p = f(...);
-  if (\(i@p1 != 0\|i@p2 < 0\))
-     return i;
-  return 0;
-...> }
-
-@s2 depends on context || org || report forall@
-identifier s1.i;
-type t;
-position q,s1.p;
-expression e,f;
-@@
-
-* t i@q;
-  ... when != i
-  e@p = f(...);
-
-@s3 depends on context || org || report@
-expression e;
-position p1!=s1.p1;
-position p2!=s1.p2;
-@@
-
-*if (\(e@p1 != 0\|e@p2 < 0\))
-   return e;
- return 0;
-
-// -----------------------------------------------------------------------
-
-@script:python depends on org@
-p << s1.p;
-p1 << s1.p1;
-q << s2.q;
-@@
-
-cocci.print_main("decl",q)
-cocci.print_secs("use",p)
-cocci.include_match(False)
-
-@script:python depends on org@
-p << s1.p;
-p2 << s1.p2;
-q << s2.q;
-@@
-
-cocci.print_main("decl",q)
-cocci.print_secs("use with questionable test",p)
-cocci.include_match(False)
-
-@script:python depends on org@
-p << s1.p;
-p1 << s1.p1;
-@@
-
-cocci.print_main("use",p)
-
-@script:python depends on org@
-p << s1.p;
-p2 << s1.p2;
-@@
-
-cocci.print_main("use with questionable test",p)
-
-@script:python depends on org@
-p << s3.p1;
-@@
-
-cocci.print_main("test",p)
-
-@script:python depends on org@
-p << s3.p2;
-@@
-
-cocci.print_main("questionable test",p)
-
-// -----------------------------------------------------------------------
-
-@script:python depends on report@
-p << s1.p;
-p1 << s1.p1;
-q << s2.q;
-@@
-
-msg = "WARNING: end returns can be simpified and declaration on line %s can be dropped" % (q[0].line)
-coccilib.report.print_report(p[0],msg)
-cocci.include_match(False)
-
-@script:python depends on report@
-p << s1.p;
-p1 << s1.p1;
-q << s2.q
-;
-@@
-
-msg = "WARNING: end returns may be simpified if negative or 0 value and declaration on line %s can be dropped" % (q[0].line)
-coccilib.report.print_report(p[0],msg)
-cocci.include_match(False)
-
-@script:python depends on report@
-p << s1.p;
-p1 << s1.p1;
-@@
-
-msg = "WARNING: end returns can be simpified"
-coccilib.report.print_report(p[0],msg)
-
-@script:python depends on report@
-p << s1.p;
-p2 << s1.p2;
-@@
-
-msg = "WARNING: end returns can be simpified if negative or 0 value"
-coccilib.report.print_report(p[0],msg)
-
-@script:python depends on report@
-p << s3.p1;
-@@
-
-msg = "WARNING: end returns can be simpified"
-coccilib.report.print_report(p[0],msg)
-
-@script:python depends on report@
-p << s3.p2;
-@@
-
-msg = "WARNING: end returns can be simpified if tested value is negative or 0"
-coccilib.report.print_report(p[0],msg)
-- 
2.4.9


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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-09-30 22:37 [PATCH] coccinelle: misc: remove "complex return code" warnings Johan Hovold
@ 2015-10-01  5:20 ` Julia Lawall
  2015-10-01 17:47   ` Johan Hovold
  2015-10-03 16:25 ` Julia Lawall
  1 sibling, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2015-10-01  5:20 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Michal Marek, Gilles Muller, Nicolas Palix, linux-kernel, cocci



On Wed, 30 Sep 2015, Johan Hovold wrote:

> This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
> to delete overly complex return code processing").
> 
> There can be both symmetry and readability reasons for not wanting to do
> the final function call as part of the return statement and to maintain
> a clear separation of success and error paths.
> 
> Since this is in no way mandated by the coding standard, let's just
> remove this semantic patch to avoid having "clean up" patches being
> posted over and over in response to these Coccinelle warnings.

What do you mean by "posted"?  Are you referring to 0-day build testing 
or individual usage of make coccicheck?  Maybe it would make sense to 
remove the semantic patch from 0-day build testing but leave it in the 
kernel, perhaps removing the < 0 case because that one in practice doesn't 
seem to turn up much that is useful?

Perhaps it could also be improved to detect a previous != 0 case and then 
not return a warning.  On some functions, this change can make some nice 
simplifications.

julia

> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  scripts/coccinelle/misc/simple_return.cocci | 180 ----------------------------
>  1 file changed, 180 deletions(-)
>  delete mode 100644 scripts/coccinelle/misc/simple_return.cocci
> 
> diff --git a/scripts/coccinelle/misc/simple_return.cocci b/scripts/coccinelle/misc/simple_return.cocci
> deleted file mode 100644
> index e8b6313b116f..000000000000
> --- a/scripts/coccinelle/misc/simple_return.cocci
> +++ /dev/null
> @@ -1,180 +0,0 @@
> -/// Simplify a trivial if-return sequence.  Possibly combine with a
> -/// preceding function call.
> -///
> -// Confidence: High
> -// Copyright: (C) 2014 Julia Lawall, INRIA/LIP6.  GPLv2.
> -// Copyright: (C) 2014 Gilles Muller, INRIA/LiP6.  GPLv2.
> -// URL: http://coccinelle.lip6.fr/
> -// Comments:
> -// Options: --no-includes --include-headers
> -
> -virtual patch
> -virtual context
> -virtual org
> -virtual report
> -
> -@r depends on patch@
> -local idexpression e;
> -identifier i,f,fn;
> -@@
> -
> -fn(...) { <...
> -- e@i =
> -+ return
> -    f(...);
> --if (i != 0) return i;
> --return 0;
> -...> }
> -
> -@depends on patch@
> -identifier r.i;
> -type t;
> -@@
> -
> --t i;
> - ... when != i
> -
> -@depends on patch@
> -expression e;
> -@@
> -
> --if (e != 0)
> -   return e;
> --return 0;
> -
> -// -----------------------------------------------------------------------
> -
> -@s1 depends on context || org || report@
> -local idexpression e;
> -identifier i,f,fn;
> -position p,p1,p2;
> -@@
> -
> -fn(...) { <...
> -* e@i@p = f(...);
> -  if (\(i@p1 != 0\|i@p2 < 0\))
> -     return i;
> -  return 0;
> -...> }
> -
> -@s2 depends on context || org || report forall@
> -identifier s1.i;
> -type t;
> -position q,s1.p;
> -expression e,f;
> -@@
> -
> -* t i@q;
> -  ... when != i
> -  e@p = f(...);
> -
> -@s3 depends on context || org || report@
> -expression e;
> -position p1!=s1.p1;
> -position p2!=s1.p2;
> -@@
> -
> -*if (\(e@p1 != 0\|e@p2 < 0\))
> -   return e;
> - return 0;
> -
> -// -----------------------------------------------------------------------
> -
> -@script:python depends on org@
> -p << s1.p;
> -p1 << s1.p1;
> -q << s2.q;
> -@@
> -
> -cocci.print_main("decl",q)
> -cocci.print_secs("use",p)
> -cocci.include_match(False)
> -
> -@script:python depends on org@
> -p << s1.p;
> -p2 << s1.p2;
> -q << s2.q;
> -@@
> -
> -cocci.print_main("decl",q)
> -cocci.print_secs("use with questionable test",p)
> -cocci.include_match(False)
> -
> -@script:python depends on org@
> -p << s1.p;
> -p1 << s1.p1;
> -@@
> -
> -cocci.print_main("use",p)
> -
> -@script:python depends on org@
> -p << s1.p;
> -p2 << s1.p2;
> -@@
> -
> -cocci.print_main("use with questionable test",p)
> -
> -@script:python depends on org@
> -p << s3.p1;
> -@@
> -
> -cocci.print_main("test",p)
> -
> -@script:python depends on org@
> -p << s3.p2;
> -@@
> -
> -cocci.print_main("questionable test",p)
> -
> -// -----------------------------------------------------------------------
> -
> -@script:python depends on report@
> -p << s1.p;
> -p1 << s1.p1;
> -q << s2.q;
> -@@
> -
> -msg = "WARNING: end returns can be simpified and declaration on line %s can be dropped" % (q[0].line)
> -coccilib.report.print_report(p[0],msg)
> -cocci.include_match(False)
> -
> -@script:python depends on report@
> -p << s1.p;
> -p1 << s1.p1;
> -q << s2.q
> -;
> -@@
> -
> -msg = "WARNING: end returns may be simpified if negative or 0 value and declaration on line %s can be dropped" % (q[0].line)
> -coccilib.report.print_report(p[0],msg)
> -cocci.include_match(False)
> -
> -@script:python depends on report@
> -p << s1.p;
> -p1 << s1.p1;
> -@@
> -
> -msg = "WARNING: end returns can be simpified"
> -coccilib.report.print_report(p[0],msg)
> -
> -@script:python depends on report@
> -p << s1.p;
> -p2 << s1.p2;
> -@@
> -
> -msg = "WARNING: end returns can be simpified if negative or 0 value"
> -coccilib.report.print_report(p[0],msg)
> -
> -@script:python depends on report@
> -p << s3.p1;
> -@@
> -
> -msg = "WARNING: end returns can be simpified"
> -coccilib.report.print_report(p[0],msg)
> -
> -@script:python depends on report@
> -p << s3.p2;
> -@@
> -
> -msg = "WARNING: end returns can be simpified if tested value is negative or 0"
> -coccilib.report.print_report(p[0],msg)
> -- 
> 2.4.9
> 
> 

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-01  5:20 ` Julia Lawall
@ 2015-10-01 17:47   ` Johan Hovold
  2015-10-02 21:33     ` Julia Lawall
  2015-10-03 16:24     ` Julia Lawall
  0 siblings, 2 replies; 11+ messages in thread
From: Johan Hovold @ 2015-10-01 17:47 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Johan Hovold, Michal Marek, Gilles Muller, Nicolas Palix,
	linux-kernel, cocci

On Thu, Oct 01, 2015 at 07:20:10AM +0200, Julia Lawall wrote:
> On Wed, 30 Sep 2015, Johan Hovold wrote:
> 
> > This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
> > to delete overly complex return code processing").
> > 
> > There can be both symmetry and readability reasons for not wanting to do
> > the final function call as part of the return statement and to maintain
> > a clear separation of success and error paths.
> > 
> > Since this is in no way mandated by the coding standard, let's just
> > remove this semantic patch to avoid having "clean up" patches being
> > posted over and over in response to these Coccinelle warnings.
> 
> What do you mean by "posted"?  Are you referring to 0-day build testing 
> or individual usage of make coccicheck?  Maybe it would make sense to 
> remove the semantic patch from 0-day build testing but leave it in the 
> kernel, perhaps removing the < 0 case because that one in practice doesn't 
> seem to turn up much that is useful?

Individuals running coccicheck on in-kernel code and posting patches to
"fix warnings", where the end result is not necessarily an improvement.

But I don't think these warnings should be enabled for 0-day build
testing either as it is should be up to the author to decide what style
to prefer in each case.

> Perhaps it could also be improved to detect a previous != 0 case and then 
> not return a warning.  On some functions, this change can make some nice 
> simplifications.

Yes, that would at least improve things.

I don't think warnings should be generated at all for the following
code:

{
	int ret;

	ret = init_a(...);
	if (ret)
		return ret;

	ret = init_b(...);
	if (ret)
		return ret;

	return 0;
}

as it is (at least to me) preferred over:

{
	int ret;

	ret = init_a(...);
	if (ret)
		return ret;

	return init_b(...);
}

for symmetry and readability reasons (e.g. I don't have to look at
init_b to figure out what the functions returns). And with a long
parameter list to init_b with line breaks, this would look even worse.

But either way, it should be up to the author of the code to decide what
style to use.

Thanks,
Johan

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-01 17:47   ` Johan Hovold
@ 2015-10-02 21:33     ` Julia Lawall
  2015-10-04 10:50       ` Johan Hovold
  2015-10-03 16:24     ` Julia Lawall
  1 sibling, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2015-10-02 21:33 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Julia Lawall, Michal Marek, Gilles Muller, Nicolas Palix,
	linux-kernel, cocci

Do you consider that this function would be better off in two lines?

static int mxt_acquire_irq(struct mxt_data *data)
{
        int error;

        enable_irq(data->irq);

        error = mxt_process_messages_until_invalid(data);
        if (error)
                return error;

        return 0;
}

Would simplifying the code at the end of the following function be helpful
or not?

static int adnp_gpio_setup(struct adnp *adnp, unsigned int num_gpios)
{
        struct gpio_chip *chip = &adnp->gpio;
        int err;

        adnp->reg_shift = get_count_order(num_gpios) - 3;

        chip->direction_input = adnp_gpio_direction_input;
        chip->direction_output = adnp_gpio_direction_output;
        chip->get = adnp_gpio_get;
        chip->set = adnp_gpio_set;
        chip->can_sleep = true;

        if (IS_ENABLED(CONFIG_DEBUG_FS))
                chip->dbg_show = adnp_gpio_dbg_show;

        chip->base = -1;
        chip->ngpio = num_gpios;
        chip->label = adnp->client->name;
        chip->dev = &adnp->client->dev;
        chip->of_node = chip->dev->of_node;
        chip->owner = THIS_MODULE;

        err = gpiochip_add(chip);
        if (err)
                return err;

        return 0;
}

thanks,
julia

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-01 17:47   ` Johan Hovold
  2015-10-02 21:33     ` Julia Lawall
@ 2015-10-03 16:24     ` Julia Lawall
  2015-10-04 10:52       ` Johan Hovold
  1 sibling, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2015-10-03 16:24 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Michal Marek, Gilles Muller, Nicolas Palix, linux-kernel, cocci

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

Perhaps there is a more restricted version that can be acceptable, but I'm
OK with dropping the current version.

julia

On Thu, 1 Oct 2015, Johan Hovold wrote:

> On Thu, Oct 01, 2015 at 07:20:10AM +0200, Julia Lawall wrote:
> > On Wed, 30 Sep 2015, Johan Hovold wrote:
> >
> > > This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
> > > to delete overly complex return code processing").
> > >
> > > There can be both symmetry and readability reasons for not wanting to do
> > > the final function call as part of the return statement and to maintain
> > > a clear separation of success and error paths.
> > >
> > > Since this is in no way mandated by the coding standard, let's just
> > > remove this semantic patch to avoid having "clean up" patches being
> > > posted over and over in response to these Coccinelle warnings.
> >
> > What do you mean by "posted"?  Are you referring to 0-day build testing
> > or individual usage of make coccicheck?  Maybe it would make sense to
> > remove the semantic patch from 0-day build testing but leave it in the
> > kernel, perhaps removing the < 0 case because that one in practice doesn't
> > seem to turn up much that is useful?
>
> Individuals running coccicheck on in-kernel code and posting patches to
> "fix warnings", where the end result is not necessarily an improvement.
>
> But I don't think these warnings should be enabled for 0-day build
> testing either as it is should be up to the author to decide what style
> to prefer in each case.
>
> > Perhaps it could also be improved to detect a previous != 0 case and then
> > not return a warning.  On some functions, this change can make some nice
> > simplifications.
>
> Yes, that would at least improve things.
>
> I don't think warnings should be generated at all for the following
> code:
>
> {
> 	int ret;
>
> 	ret = init_a(...);
> 	if (ret)
> 		return ret;
>
> 	ret = init_b(...);
> 	if (ret)
> 		return ret;
>
> 	return 0;
> }
>
> as it is (at least to me) preferred over:
>
> {
> 	int ret;
>
> 	ret = init_a(...);
> 	if (ret)
> 		return ret;
>
> 	return init_b(...);
> }
>
> for symmetry and readability reasons (e.g. I don't have to look at
> init_b to figure out what the functions returns). And with a long
> parameter list to init_b with line breaks, this would look even worse.
>
> But either way, it should be up to the author of the code to decide what
> style to use.
>
> Thanks,
> Johan
>

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-09-30 22:37 [PATCH] coccinelle: misc: remove "complex return code" warnings Johan Hovold
  2015-10-01  5:20 ` Julia Lawall
@ 2015-10-03 16:25 ` Julia Lawall
  2015-10-28  9:54   ` Johan Hovold
  1 sibling, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2015-10-03 16:25 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Julia Lawall, Michal Marek, Gilles Muller, Nicolas Palix,
	linux-kernel, cocci

Should have acked this message...

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

On Wed, 30 Sep 2015, Johan Hovold wrote:

> This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
> to delete overly complex return code processing").
>
> There can be both symmetry and readability reasons for not wanting to do
> the final function call as part of the return statement and to maintain
> a clear separation of success and error paths.
>
> Since this is in no way mandated by the coding standard, let's just
> remove this semantic patch to avoid having "clean up" patches being
> posted over and over in response to these Coccinelle warnings.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  scripts/coccinelle/misc/simple_return.cocci | 180 ----------------------------
>  1 file changed, 180 deletions(-)
>  delete mode 100644 scripts/coccinelle/misc/simple_return.cocci
>
> diff --git a/scripts/coccinelle/misc/simple_return.cocci b/scripts/coccinelle/misc/simple_return.cocci
> deleted file mode 100644
> index e8b6313b116f..000000000000
> --- a/scripts/coccinelle/misc/simple_return.cocci
> +++ /dev/null
> @@ -1,180 +0,0 @@
> -/// Simplify a trivial if-return sequence.  Possibly combine with a
> -/// preceding function call.
> -///
> -// Confidence: High
> -// Copyright: (C) 2014 Julia Lawall, INRIA/LIP6.  GPLv2.
> -// Copyright: (C) 2014 Gilles Muller, INRIA/LiP6.  GPLv2.
> -// URL: http://coccinelle.lip6.fr/
> -// Comments:
> -// Options: --no-includes --include-headers
> -
> -virtual patch
> -virtual context
> -virtual org
> -virtual report
> -
> -@r depends on patch@
> -local idexpression e;
> -identifier i,f,fn;
> -@@
> -
> -fn(...) { <...
> -- e@i =
> -+ return
> -    f(...);
> --if (i != 0) return i;
> --return 0;
> -...> }
> -
> -@depends on patch@
> -identifier r.i;
> -type t;
> -@@
> -
> --t i;
> - ... when != i
> -
> -@depends on patch@
> -expression e;
> -@@
> -
> --if (e != 0)
> -   return e;
> --return 0;
> -
> -// -----------------------------------------------------------------------
> -
> -@s1 depends on context || org || report@
> -local idexpression e;
> -identifier i,f,fn;
> -position p,p1,p2;
> -@@
> -
> -fn(...) { <...
> -* e@i@p = f(...);
> -  if (\(i@p1 != 0\|i@p2 < 0\))
> -     return i;
> -  return 0;
> -...> }
> -
> -@s2 depends on context || org || report forall@
> -identifier s1.i;
> -type t;
> -position q,s1.p;
> -expression e,f;
> -@@
> -
> -* t i@q;
> -  ... when != i
> -  e@p = f(...);
> -
> -@s3 depends on context || org || report@
> -expression e;
> -position p1!=s1.p1;
> -position p2!=s1.p2;
> -@@
> -
> -*if (\(e@p1 != 0\|e@p2 < 0\))
> -   return e;
> - return 0;
> -
> -// -----------------------------------------------------------------------
> -
> -@script:python depends on org@
> -p << s1.p;
> -p1 << s1.p1;
> -q << s2.q;
> -@@
> -
> -cocci.print_main("decl",q)
> -cocci.print_secs("use",p)
> -cocci.include_match(False)
> -
> -@script:python depends on org@
> -p << s1.p;
> -p2 << s1.p2;
> -q << s2.q;
> -@@
> -
> -cocci.print_main("decl",q)
> -cocci.print_secs("use with questionable test",p)
> -cocci.include_match(False)
> -
> -@script:python depends on org@
> -p << s1.p;
> -p1 << s1.p1;
> -@@
> -
> -cocci.print_main("use",p)
> -
> -@script:python depends on org@
> -p << s1.p;
> -p2 << s1.p2;
> -@@
> -
> -cocci.print_main("use with questionable test",p)
> -
> -@script:python depends on org@
> -p << s3.p1;
> -@@
> -
> -cocci.print_main("test",p)
> -
> -@script:python depends on org@
> -p << s3.p2;
> -@@
> -
> -cocci.print_main("questionable test",p)
> -
> -// -----------------------------------------------------------------------
> -
> -@script:python depends on report@
> -p << s1.p;
> -p1 << s1.p1;
> -q << s2.q;
> -@@
> -
> -msg = "WARNING: end returns can be simpified and declaration on line %s can be dropped" % (q[0].line)
> -coccilib.report.print_report(p[0],msg)
> -cocci.include_match(False)
> -
> -@script:python depends on report@
> -p << s1.p;
> -p1 << s1.p1;
> -q << s2.q
> -;
> -@@
> -
> -msg = "WARNING: end returns may be simpified if negative or 0 value and declaration on line %s can be dropped" % (q[0].line)
> -coccilib.report.print_report(p[0],msg)
> -cocci.include_match(False)
> -
> -@script:python depends on report@
> -p << s1.p;
> -p1 << s1.p1;
> -@@
> -
> -msg = "WARNING: end returns can be simpified"
> -coccilib.report.print_report(p[0],msg)
> -
> -@script:python depends on report@
> -p << s1.p;
> -p2 << s1.p2;
> -@@
> -
> -msg = "WARNING: end returns can be simpified if negative or 0 value"
> -coccilib.report.print_report(p[0],msg)
> -
> -@script:python depends on report@
> -p << s3.p1;
> -@@
> -
> -msg = "WARNING: end returns can be simpified"
> -coccilib.report.print_report(p[0],msg)
> -
> -@script:python depends on report@
> -p << s3.p2;
> -@@
> -
> -msg = "WARNING: end returns can be simpified if tested value is negative or 0"
> -coccilib.report.print_report(p[0],msg)
> --
> 2.4.9
>
>

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-02 21:33     ` Julia Lawall
@ 2015-10-04 10:50       ` Johan Hovold
  0 siblings, 0 replies; 11+ messages in thread
From: Johan Hovold @ 2015-10-04 10:50 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Johan Hovold, Michal Marek, Gilles Muller, Nicolas Palix,
	linux-kernel, cocci

On Fri, Oct 02, 2015 at 11:33:46PM +0200, Julia Lawall wrote:
> Do you consider that this function would be better off in two lines?
> 
> static int mxt_acquire_irq(struct mxt_data *data)
> {
>         int error;
> 
>         enable_irq(data->irq);
> 
>         error = mxt_process_messages_until_invalid(data);
>         if (error)
>                 return error;
> 
>         return 0;
> }

Actually no, but again I'd say it's up to the author to decide.

> Would simplifying the code at the end of the following function be helpful
> or not?
> 
> static int adnp_gpio_setup(struct adnp *adnp, unsigned int num_gpios)
> {
>         struct gpio_chip *chip = &adnp->gpio;
>         int err;
> 
>         adnp->reg_shift = get_count_order(num_gpios) - 3;
> 
>         chip->direction_input = adnp_gpio_direction_input;
>         chip->direction_output = adnp_gpio_direction_output;
>         chip->get = adnp_gpio_get;
>         chip->set = adnp_gpio_set;
>         chip->can_sleep = true;
> 
>         if (IS_ENABLED(CONFIG_DEBUG_FS))
>                 chip->dbg_show = adnp_gpio_dbg_show;
> 
>         chip->base = -1;
>         chip->ngpio = num_gpios;
>         chip->label = adnp->client->name;
>         chip->dev = &adnp->client->dev;
>         chip->of_node = chip->dev->of_node;
>         chip->owner = THIS_MODULE;
> 
>         err = gpiochip_add(chip);
>         if (err)
>                 return err;
> 
>         return 0;
> }

I think this is just fine as is as well.

Thanks,
Johan

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-03 16:24     ` Julia Lawall
@ 2015-10-04 10:52       ` Johan Hovold
  0 siblings, 0 replies; 11+ messages in thread
From: Johan Hovold @ 2015-10-04 10:52 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Johan Hovold, Michal Marek, Gilles Muller, Nicolas Palix,
	linux-kernel, cocci

On Sat, Oct 03, 2015 at 06:24:27PM +0200, Julia Lawall wrote:
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> 
> Perhaps there is a more restricted version that can be acceptable, but I'm
> OK with dropping the current version.

Great, thanks!

Johan

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-03 16:25 ` Julia Lawall
@ 2015-10-28  9:54   ` Johan Hovold
  2015-10-28 10:04     ` Michal Marek
  0 siblings, 1 reply; 11+ messages in thread
From: Johan Hovold @ 2015-10-28  9:54 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Johan Hovold, Michal Marek, Gilles Muller, Nicolas Palix,
	linux-kernel, cocci

On Sat, Oct 03, 2015 at 06:25:39PM +0200, Julia Lawall wrote:
> Should have acked this message...
> 
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> 
> On Wed, 30 Sep 2015, Johan Hovold wrote:
> 
> > This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
> > to delete overly complex return code processing").
> >
> > There can be both symmetry and readability reasons for not wanting to do
> > the final function call as part of the return statement and to maintain
> > a clear separation of success and error paths.
> >
> > Since this is in no way mandated by the coding standard, let's just
> > remove this semantic patch to avoid having "clean up" patches being
> > posted over and over in response to these Coccinelle warnings.
> >
> > Signed-off-by: Johan Hovold <johan@kernel.org>

Is anyone planning on picking this patch up?

Thanks,
Johan

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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-28  9:54   ` Johan Hovold
@ 2015-10-28 10:04     ` Michal Marek
  2015-10-28 10:09       ` Johan Hovold
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Marek @ 2015-10-28 10:04 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Julia Lawall, Gilles Muller, Nicolas Palix, linux-kernel, cocci

Dne 28.10.2015 v 10:54 Johan Hovold napsal(a):
> On Sat, Oct 03, 2015 at 06:25:39PM +0200, Julia Lawall wrote:
>> Should have acked this message...
>>
>> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>>
>> On Wed, 30 Sep 2015, Johan Hovold wrote:
>>
>>> This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
>>> to delete overly complex return code processing").
>>>
>>> There can be both symmetry and readability reasons for not wanting to do
>>> the final function call as part of the return statement and to maintain
>>> a clear separation of success and error paths.
>>>
>>> Since this is in no way mandated by the coding standard, let's just
>>> remove this semantic patch to avoid having "clean up" patches being
>>> posted over and over in response to these Coccinelle warnings.
>>>
>>> Signed-off-by: Johan Hovold <johan@kernel.org>
> 
> Is anyone planning on picking this patch up?

It is already in kbuild.git as commit 1a617a8475e8, I apparenly forgot
to reply to this thread.

Michal


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

* Re: [PATCH] coccinelle: misc: remove "complex return code" warnings
  2015-10-28 10:04     ` Michal Marek
@ 2015-10-28 10:09       ` Johan Hovold
  0 siblings, 0 replies; 11+ messages in thread
From: Johan Hovold @ 2015-10-28 10:09 UTC (permalink / raw)
  To: Michal Marek
  Cc: Johan Hovold, Julia Lawall, Gilles Muller, Nicolas Palix,
	linux-kernel, cocci

On Wed, Oct 28, 2015 at 11:04:56AM +0100, Michal Marek wrote:
> Dne 28.10.2015 v 10:54 Johan Hovold napsal(a):
> > On Sat, Oct 03, 2015 at 06:25:39PM +0200, Julia Lawall wrote:
> >> Should have acked this message...
> >>
> >> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> >>
> >> On Wed, 30 Sep 2015, Johan Hovold wrote:
> >>
> >>> This effectively reverts 932058a5d5f9 ("coccinelle: misc: semantic patch
> >>> to delete overly complex return code processing").
> >>>
> >>> There can be both symmetry and readability reasons for not wanting to do
> >>> the final function call as part of the return statement and to maintain
> >>> a clear separation of success and error paths.
> >>>
> >>> Since this is in no way mandated by the coding standard, let's just
> >>> remove this semantic patch to avoid having "clean up" patches being
> >>> posted over and over in response to these Coccinelle warnings.
> >>>
> >>> Signed-off-by: Johan Hovold <johan@kernel.org>
> > 
> > Is anyone planning on picking this patch up?
> 
> It is already in kbuild.git as commit 1a617a8475e8, I apparenly forgot
> to reply to this thread.

Ok, great.

Thanks,
Johan

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

end of thread, other threads:[~2015-10-28 10:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-30 22:37 [PATCH] coccinelle: misc: remove "complex return code" warnings Johan Hovold
2015-10-01  5:20 ` Julia Lawall
2015-10-01 17:47   ` Johan Hovold
2015-10-02 21:33     ` Julia Lawall
2015-10-04 10:50       ` Johan Hovold
2015-10-03 16:24     ` Julia Lawall
2015-10-04 10:52       ` Johan Hovold
2015-10-03 16:25 ` Julia Lawall
2015-10-28  9:54   ` Johan Hovold
2015-10-28 10:04     ` Michal Marek
2015-10-28 10:09       ` Johan Hovold

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).