| Message ID | 1385991581-16843-1-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 2013-12-02 14:39, Nishanth Menon wrote: > As indicated by Sekhar in [1], there seems to be a tendency to use > IS_ERR_VALUE to check the error result for pm_runtime_* functions which > make no sense considering commit c48cd65 (ARM: OMAP: use consistent > error checking) - the error values can either be < 0 for error OR > 0, 1 in cases where we have success. > > So, setup a coccinelle script to help identify the same. Julia, Gilles, do you have any objections to this semantic patch? Thanks, Michal > > [1] http://marc.info/?t=138472678100003&r=1&w=2 > > Cc: Russell King <rmk+kernel@arm.linux.org.uk> > Reported-by: Sekhar Nori <nsekhar@ti.com> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > Original post: http://marc.info/?t=138559437300001&r=1&w=2 > > scripts/coccinelle/api/pm_runtime.cocci | 109 +++++++++++++++++++++++++++++++ > 1 file changed, 109 insertions(+) > create mode 100644 scripts/coccinelle/api/pm_runtime.cocci > > diff --git a/scripts/coccinelle/api/pm_runtime.cocci b/scripts/coccinelle/api/pm_runtime.cocci > new file mode 100644 > index 0000000..f01789e > --- /dev/null > +++ b/scripts/coccinelle/api/pm_runtime.cocci > @@ -0,0 +1,109 @@ > +/// Make sure pm_runtime_* calls does not use unnecessary IS_ERR_VALUE > +// > +// Keywords: pm_runtime > +// Confidence: Medium > +// Copyright (C) 2013 Texas Instruments Incorporated - GPLv2. > +// URL: http://coccinelle.lip6.fr/ > +// Options: --include-headers > + > +virtual patch > +virtual context > +virtual org > +virtual report > + > +//---------------------------------------------------------- > +// Detection > +//---------------------------------------------------------- > + > +@runtime_bad_err_handle exists@ > +expression ret; > +@@ > +( > +ret = \(pm_runtime_idle\| > + pm_runtime_suspend\| > + pm_runtime_autosuspend\| > + pm_runtime_resume\| > + pm_request_idle\| > + pm_request_resume\| > + pm_request_autosuspend\| > + pm_runtime_get\| > + pm_runtime_get_sync\| > + pm_runtime_put\| > + pm_runtime_put_autosuspend\| > + pm_runtime_put_sync\| > + pm_runtime_put_sync_suspend\| > + pm_runtime_put_sync_autosuspend\| > + pm_runtime_set_active\| > + pm_schedule_suspend\| > + pm_runtime_barrier\| > + pm_generic_runtime_suspend\| > + pm_generic_runtime_resume\)(...); > +... > +IS_ERR_VALUE(ret) > +... > +) > + > +//---------------------------------------------------------- > +// For context mode > +//---------------------------------------------------------- > + > +@depends on runtime_bad_err_handle && context@ > +identifier pm_runtime_api; > +expression ret; > +@@ > +( > +ret = pm_runtime_api(...); > +... > +* IS_ERR_VALUE(ret) > +... > +) > + > +//---------------------------------------------------------- > +// For patch mode > +//---------------------------------------------------------- > + > +@depends on runtime_bad_err_handle && patch@ > +identifier pm_runtime_api; > +expression ret; > +@@ > +( > +ret = pm_runtime_api(...); > +... > +- IS_ERR_VALUE(ret) > ++ ret < 0 > +... > +) > + > +//---------------------------------------------------------- > +// For org and report mode > +//---------------------------------------------------------- > + > +@r depends on runtime_bad_err_handle exists@ > +position p1, p2; > +identifier pm_runtime_api; > +expression ret; > +@@ > +( > +ret = pm_runtime_api@p1(...); > +... > +IS_ERR_VALUE@p2(ret) > +... > +) > + > +@script:python depends on org@ > +p1 << r.p1; > +p2 << r.p2; > +pm_runtime_api << r.pm_runtime_api; > +@@ > + > +cocci.print_main(pm_runtime_api,p1) > +cocci.print_secs("IS_ERR_VALUE",p2) > + > +@script:python depends on report@ > +p1 << r.p1; > +p2 << r.p2; > +pm_runtime_api << r.pm_runtime_api; > +@@ > + > +msg = "%s returns < 0 as error. Unecessary IS_ERR_VALUE at line %s" % (pm_runtime_api, p2[0].line) > +coccilib.report.print_report(p1[0],msg) > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 3 Jan 2014, Michal Marek wrote: > On 2013-12-02 14:39, Nishanth Menon wrote: > > As indicated by Sekhar in [1], there seems to be a tendency to use > > IS_ERR_VALUE to check the error result for pm_runtime_* functions which > > make no sense considering commit c48cd65 (ARM: OMAP: use consistent > > error checking) - the error values can either be < 0 for error OR > > 0, 1 in cases where we have success. > > > > So, setup a coccinelle script to help identify the same. > > Julia, Gilles, do you have any objections to this semantic patch? No objections. Acked-by: Julia Lawall <julia.lawall@lip6.fr> > > Thanks, > Michal > > > > [1] http://marc.info/?t=138472678100003&r=1&w=2 > > > > Cc: Russell King <rmk+kernel@arm.linux.org.uk> > > Reported-by: Sekhar Nori <nsekhar@ti.com> > > Signed-off-by: Nishanth Menon <nm@ti.com> > > --- > > Original post: http://marc.info/?t=138559437300001&r=1&w=2 > > > > scripts/coccinelle/api/pm_runtime.cocci | 109 +++++++++++++++++++++++++++++++ > > 1 file changed, 109 insertions(+) > > create mode 100644 scripts/coccinelle/api/pm_runtime.cocci > > > > diff --git a/scripts/coccinelle/api/pm_runtime.cocci b/scripts/coccinelle/api/pm_runtime.cocci > > new file mode 100644 > > index 0000000..f01789e > > --- /dev/null > > +++ b/scripts/coccinelle/api/pm_runtime.cocci > > @@ -0,0 +1,109 @@ > > +/// Make sure pm_runtime_* calls does not use unnecessary IS_ERR_VALUE > > +// > > +// Keywords: pm_runtime > > +// Confidence: Medium > > +// Copyright (C) 2013 Texas Instruments Incorporated - GPLv2. > > +// URL: http://coccinelle.lip6.fr/ > > +// Options: --include-headers > > + > > +virtual patch > > +virtual context > > +virtual org > > +virtual report > > + > > +//---------------------------------------------------------- > > +// Detection > > +//---------------------------------------------------------- > > + > > +@runtime_bad_err_handle exists@ > > +expression ret; > > +@@ > > +( > > +ret = \(pm_runtime_idle\| > > + pm_runtime_suspend\| > > + pm_runtime_autosuspend\| > > + pm_runtime_resume\| > > + pm_request_idle\| > > + pm_request_resume\| > > + pm_request_autosuspend\| > > + pm_runtime_get\| > > + pm_runtime_get_sync\| > > + pm_runtime_put\| > > + pm_runtime_put_autosuspend\| > > + pm_runtime_put_sync\| > > + pm_runtime_put_sync_suspend\| > > + pm_runtime_put_sync_autosuspend\| > > + pm_runtime_set_active\| > > + pm_schedule_suspend\| > > + pm_runtime_barrier\| > > + pm_generic_runtime_suspend\| > > + pm_generic_runtime_resume\)(...); > > +... > > +IS_ERR_VALUE(ret) > > +... > > +) > > + > > +//---------------------------------------------------------- > > +// For context mode > > +//---------------------------------------------------------- > > + > > +@depends on runtime_bad_err_handle && context@ > > +identifier pm_runtime_api; > > +expression ret; > > +@@ > > +( > > +ret = pm_runtime_api(...); > > +... > > +* IS_ERR_VALUE(ret) > > +... > > +) > > + > > +//---------------------------------------------------------- > > +// For patch mode > > +//---------------------------------------------------------- > > + > > +@depends on runtime_bad_err_handle && patch@ > > +identifier pm_runtime_api; > > +expression ret; > > +@@ > > +( > > +ret = pm_runtime_api(...); > > +... > > +- IS_ERR_VALUE(ret) > > ++ ret < 0 > > +... > > +) > > + > > +//---------------------------------------------------------- > > +// For org and report mode > > +//---------------------------------------------------------- > > + > > +@r depends on runtime_bad_err_handle exists@ > > +position p1, p2; > > +identifier pm_runtime_api; > > +expression ret; > > +@@ > > +( > > +ret = pm_runtime_api@p1(...); > > +... > > +IS_ERR_VALUE@p2(ret) > > +... > > +) > > + > > +@script:python depends on org@ > > +p1 << r.p1; > > +p2 << r.p2; > > +pm_runtime_api << r.pm_runtime_api; > > +@@ > > + > > +cocci.print_main(pm_runtime_api,p1) > > +cocci.print_secs("IS_ERR_VALUE",p2) > > + > > +@script:python depends on report@ > > +p1 << r.p1; > > +p2 << r.p2; > > +pm_runtime_api << r.pm_runtime_api; > > +@@ > > + > > +msg = "%s returns < 0 as error. Unecessary IS_ERR_VALUE at line %s" % (pm_runtime_api, p2[0].line) > > +coccilib.report.print_report(p1[0],msg) > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 03, 2014 at 02:29:35PM +0100, Julia Lawall wrote: > > On Fri, 3 Jan 2014, Michal Marek wrote: > > > On 2013-12-02 14:39, Nishanth Menon wrote: > > > As indicated by Sekhar in [1], there seems to be a tendency to use > > > IS_ERR_VALUE to check the error result for pm_runtime_* functions which > > > make no sense considering commit c48cd65 (ARM: OMAP: use consistent > > > error checking) - the error values can either be < 0 for error OR > > > 0, 1 in cases where we have success. > > > > > > So, setup a coccinelle script to help identify the same. > > > > Julia, Gilles, do you have any objections to this semantic patch? > > No objections. > > Acked-by: Julia Lawall <julia.lawall@lip6.fr> Thanks, applied to kbuild.git#misc. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/coccinelle/api/pm_runtime.cocci b/scripts/coccinelle/api/pm_runtime.cocci new file mode 100644 index 0000000..f01789e --- /dev/null +++ b/scripts/coccinelle/api/pm_runtime.cocci @@ -0,0 +1,109 @@ +/// Make sure pm_runtime_* calls does not use unnecessary IS_ERR_VALUE +// +// Keywords: pm_runtime +// Confidence: Medium +// Copyright (C) 2013 Texas Instruments Incorporated - GPLv2. +// URL: http://coccinelle.lip6.fr/ +// Options: --include-headers + +virtual patch +virtual context +virtual org +virtual report + +//---------------------------------------------------------- +// Detection +//---------------------------------------------------------- + +@runtime_bad_err_handle exists@ +expression ret; +@@ +( +ret = \(pm_runtime_idle\| + pm_runtime_suspend\| + pm_runtime_autosuspend\| + pm_runtime_resume\| + pm_request_idle\| + pm_request_resume\| + pm_request_autosuspend\| + pm_runtime_get\| + pm_runtime_get_sync\| + pm_runtime_put\| + pm_runtime_put_autosuspend\| + pm_runtime_put_sync\| + pm_runtime_put_sync_suspend\| + pm_runtime_put_sync_autosuspend\| + pm_runtime_set_active\| + pm_schedule_suspend\| + pm_runtime_barrier\| + pm_generic_runtime_suspend\| + pm_generic_runtime_resume\)(...); +... +IS_ERR_VALUE(ret) +... +) + +//---------------------------------------------------------- +// For context mode +//---------------------------------------------------------- + +@depends on runtime_bad_err_handle && context@ +identifier pm_runtime_api; +expression ret; +@@ +( +ret = pm_runtime_api(...); +... +* IS_ERR_VALUE(ret) +... +) + +//---------------------------------------------------------- +// For patch mode +//---------------------------------------------------------- + +@depends on runtime_bad_err_handle && patch@ +identifier pm_runtime_api; +expression ret; +@@ +( +ret = pm_runtime_api(...); +... +- IS_ERR_VALUE(ret) ++ ret < 0 +... +) + +//---------------------------------------------------------- +// For org and report mode +//---------------------------------------------------------- + +@r depends on runtime_bad_err_handle exists@ +position p1, p2; +identifier pm_runtime_api; +expression ret; +@@ +( +ret = pm_runtime_api@p1(...); +... +IS_ERR_VALUE@p2(ret) +... +) + +@script:python depends on org@ +p1 << r.p1; +p2 << r.p2; +pm_runtime_api << r.pm_runtime_api; +@@ + +cocci.print_main(pm_runtime_api,p1) +cocci.print_secs("IS_ERR_VALUE",p2) + +@script:python depends on report@ +p1 << r.p1; +p2 << r.p2; +pm_runtime_api << r.pm_runtime_api; +@@ + +msg = "%s returns < 0 as error. Unecessary IS_ERR_VALUE at line %s" % (pm_runtime_api, p2[0].line) +coccilib.report.print_report(p1[0],msg)
As indicated by Sekhar in [1], there seems to be a tendency to use IS_ERR_VALUE to check the error result for pm_runtime_* functions which make no sense considering commit c48cd65 (ARM: OMAP: use consistent error checking) - the error values can either be < 0 for error OR 0, 1 in cases where we have success. So, setup a coccinelle script to help identify the same. [1] http://marc.info/?t=138472678100003&r=1&w=2 Cc: Russell King <rmk+kernel@arm.linux.org.uk> Reported-by: Sekhar Nori <nsekhar@ti.com> Signed-off-by: Nishanth Menon <nm@ti.com> --- Original post: http://marc.info/?t=138559437300001&r=1&w=2 scripts/coccinelle/api/pm_runtime.cocci | 109 +++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 scripts/coccinelle/api/pm_runtime.cocci