* [PATCH v2] coccinelle: Detect clk_register() anti-pattern
@ 2026-08-04 6:20 Guru Das Srinagesh
2026-08-09 20:00 ` Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Guru Das Srinagesh @ 2026-08-04 6:20 UTC (permalink / raw)
To: Julia Lawall, Nicolas Palix, Michael Turquette, Stephen Boyd
Cc: linux-kernel, cocci, Brian Masney, linux-clk, Guru Das Srinagesh
Enforce commit 12a0fd23e870 ("clk: Print an error when clk registration
fails"): clk_register(), clk_hw_register(), and their devm_/of_ variants
log their own error on failure, so driver-side error prints after these
calls are redundant.
Two independent match families, one per return-value convention:
pointer return checked via IS_ERR() (clk_register()/devm_clk_register()),
and int return checked via a nonzero value (clk_hw_register()/
devm_clk_hw_register()/of_clk_hw_register()).
In "patch" mode, removing the redundant message also collapses the
enclosing braces when only one statement remains, and deletes the whole
"if" when the message was already the only (braceless) statement.
Assisted-by: Claude:claude-sonnet-5 coccinelle
Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
---
Add a Coccinelle semantic patch enforcing commit 12a0fd23e870 ("clk:
Print an error when clk registration fails"): flags, and in "patch"
mode removes, driver-side error prints that are now redundant after
clk_register()/clk_hw_register() and their devm_/of_ variants.
Two independent match families, one per return-value convention.
Pointer return, IS_ERR()-checked (clk_register()/devm_clk_register()),
e.g. drivers/clk/clk-xgene.c:152-157:
clk = clk_register(dev, &apmclk->hw);
if (IS_ERR(clk)) {
- pr_err("%s: could not register clk %s\n", __func__, name);
kfree(apmclk);
return NULL;
}
Int return, nonzero-checked (clk_hw_register()/devm_clk_hw_register()/
of_clk_hw_register()), e.g. drivers/clk/meson/meson-clkc-utils.c:49-54:
ret = devm_clk_hw_register(dev, hw);
- if (ret) {
- dev_err(dev, "registering %s clock failed\n",
- hw->init->name);
+ if (ret)
return ret;
- }
Already-braceless single-statement case: the whole "if" is deleted
instead of just the message, e.g. drivers/clk/ux500/clk-sysctrl.c:171-175:
clk_reg = devm_clk_register(clk->dev, &clk->hw);
- if (IS_ERR(clk_reg))
- dev_err(dev, "clk_sysctrl: clk_register failed\n");
return clk_reg;
Testing:
- Baseline: coccinelle 1.3.1, the Torvalds tree at v7.2-rc5.
- "make coccicheck COCCI=<path> MODE=report M=drivers/clk" produced 73
hits and verified to have zero false positives.
- "MODE=patch" verified separately on scratch copies of affected files
to confirm minimal, correct diffs.
checkpatch flagged that this new file needs a MAINTAINERS entry, and I'd
like to maintain it, so this adds a standalone entry rather than leaving
the file uncovered. There's no direct precedent for an individual .cocci
file getting its own entry - the only other named .cocci file in
MAINTAINERS, scripts/coccinelle/api/string_choices.cocci, was added to
the existing GENERIC STRING LIBRARY entry by that subsystem's
maintainer, not as a new one. Happy to fold this into COMMON CLK
FRAMEWORK or drop it entirely depending on what the maintainers prefer.
---
Changes in v2 (Julia):
- Use a literal function-name disjunction instead of a regex identifier,
enabling spatch's file pre-filter optimization.
- In "patch" mode, drop braces when only one statement remains, and
delete the whole "if" when the message was the only (braceless)
statement.
- Drop two never-observed condition variants (IS_ERR(clk) == 1, ret !=
0); keep the one with real precedent (ret < 0).
- Link to v1: https://patch.msgid.link/20260802-cocci-clk-register-v1-1-df68afcb1eef@gurudas.dev
---
MAINTAINERS | 5 +
scripts/coccinelle/api/clk_register.cocci | 153 ++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 716acfc3d7c1..26788ccbf98c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6363,6 +6363,11 @@ L: linux-clk@vger.kernel.org
S: Maintained
F: include/linux/clk.h
+CLK_REGISTER() COCCINELLE CHECK
+M: Guru Das Srinagesh <linux@gurudas.dev>
+S: Maintained
+F: scripts/coccinelle/api/clk_register.cocci
+
CLOCKSOURCE, CLOCKEVENT DRIVERS
M: Daniel Lezcano <daniel.lezcano@kernel.org>
M: Thomas Gleixner <tglx@kernel.org>
diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
new file mode 100644
index 000000000000..86a8be92f2a2
--- /dev/null
+++ b/scripts/coccinelle/api/clk_register.cocci
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+/// Remove error messages after clk registration failures, because
+/// clk_register(), clk_hw_register(), and their variants already log
+/// an error when they fail. See commit 12a0fd23e870 ("clk: Print an
+/// error when clk registration fails").
+//
+// Confidence: Medium
+// Options: --include-headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@depends on context@
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+{
+...
+*voidfn(...);
+...
+}
+
+@depends on patch@
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+-if ( IS_ERR(clk) )
+-voidfn(...);
+
+@depends on patch@
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S;
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+(
+-{
+-voidfn(...);
+S
+-}
+|
+{
+...
+-voidfn(...);
+...
+}
+)
+
+@r1 depends on org || report@
+position p1;
+expression clk;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+clk = \(clk_register\|devm_clk_register\)(...);
+if ( IS_ERR(clk) )
+{
+...
+voidfn@p1(...);
+...
+}
+
+@depends on context@
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+{
+...
+*voidfn(...);
+...
+}
+
+@depends on patch@
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+-if ( \( ret \| ret < 0 \) )
+-voidfn(...);
+
+@depends on patch@
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+statement S;
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+(
+-{
+-voidfn(...);
+S
+-}
+|
+{
+...
+-voidfn(...);
+...
+}
+)
+
+@r2 depends on org || report@
+position p2;
+expression ret;
+identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
+@@
+
+ret = \(clk_hw_register\|devm_clk_hw_register\|of_clk_hw_register\)(...);
+if ( \( ret \| ret < 0 \) )
+{
+...
+voidfn@p2(...);
+...
+}
+
+@script:python depends on org@
+p1 << r1.p1;
+@@
+
+cocci.print_main(p1)
+
+@script:python depends on report@
+p1 << r1.p1;
+@@
+
+msg = "line %s is redundant because clk_register() already prints an error on failure" % (p1[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p2 << r2.p2;
+@@
+
+cocci.print_main(p2)
+
+@script:python depends on report@
+p2 << r2.p2;
+@@
+
+msg = "line %s is redundant because clk_hw_register() already prints an error on failure" % (p2[0].line)
+coccilib.report.print_report(p2[0], msg)
---
base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
change-id: 20260802-cocci-clk-register-951d94251af4
Best regards,
--
Guru Das Srinagesh <linux@gurudas.dev>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] coccinelle: Detect clk_register() anti-pattern
2026-08-04 6:20 [PATCH v2] coccinelle: Detect clk_register() anti-pattern Guru Das Srinagesh
@ 2026-08-09 20:00 ` Julia Lawall
2026-08-10 6:30 ` Guru Das Srinagesh
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2026-08-09 20:00 UTC (permalink / raw)
To: Guru Das Srinagesh
Cc: Julia Lawall, Nicolas Palix, Michael Turquette, Stephen Boyd,
linux-kernel, cocci, Brian Masney, linux-clk
On Mon, 3 Aug 2026, Guru Das Srinagesh wrote:
> Enforce commit 12a0fd23e870 ("clk: Print an error when clk registration
> fails"): clk_register(), clk_hw_register(), and their devm_/of_ variants
> log their own error on failure, so driver-side error prints after these
> calls are redundant.
>
> Two independent match families, one per return-value convention:
> pointer return checked via IS_ERR() (clk_register()/devm_clk_register()),
> and int return checked via a nonzero value (clk_hw_register()/
> devm_clk_hw_register()/of_clk_hw_register()).
>
> In "patch" mode, removing the redundant message also collapses the
> enclosing braces when only one statement remains, and deletes the whole
> "if" when the message was already the only (braceless) statement.
>
> Assisted-by: Claude:claude-sonnet-5 coccinelle
> Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
> ---
> Add a Coccinelle semantic patch enforcing commit 12a0fd23e870 ("clk:
> Print an error when clk registration fails"): flags, and in "patch"
> mode removes, driver-side error prints that are now redundant after
> clk_register()/clk_hw_register() and their devm_/of_ variants.
>
> Two independent match families, one per return-value convention.
>
> Pointer return, IS_ERR()-checked (clk_register()/devm_clk_register()),
> e.g. drivers/clk/clk-xgene.c:152-157:
>
> clk = clk_register(dev, &apmclk->hw);
> if (IS_ERR(clk)) {
> - pr_err("%s: could not register clk %s\n", __func__, name);
> kfree(apmclk);
> return NULL;
> }
>
> Int return, nonzero-checked (clk_hw_register()/devm_clk_hw_register()/
> of_clk_hw_register()), e.g. drivers/clk/meson/meson-clkc-utils.c:49-54:
>
> ret = devm_clk_hw_register(dev, hw);
> - if (ret) {
> - dev_err(dev, "registering %s clock failed\n",
> - hw->init->name);
> + if (ret)
> return ret;
> - }
>
> Already-braceless single-statement case: the whole "if" is deleted
> instead of just the message, e.g. drivers/clk/ux500/clk-sysctrl.c:171-175:
>
> clk_reg = devm_clk_register(clk->dev, &clk->hw);
> - if (IS_ERR(clk_reg))
> - dev_err(dev, "clk_sysctrl: clk_register failed\n");
>
> return clk_reg;
>
> Testing:
> - Baseline: coccinelle 1.3.1, the Torvalds tree at v7.2-rc5.
> - "make coccicheck COCCI=<path> MODE=report M=drivers/clk" produced 73
> hits and verified to have zero false positives.
> - "MODE=patch" verified separately on scratch copies of affected files
> to confirm minimal, correct diffs.
>
> checkpatch flagged that this new file needs a MAINTAINERS entry, and I'd
> like to maintain it, so this adds a standalone entry rather than leaving
> the file uncovered. There's no direct precedent for an individual .cocci
> file getting its own entry - the only other named .cocci file in
> MAINTAINERS, scripts/coccinelle/api/string_choices.cocci, was added to
> the existing GENERIC STRING LIBRARY entry by that subsystem's
> maintainer, not as a new one. Happy to fold this into COMMON CLK
> FRAMEWORK or drop it entirely depending on what the maintainers prefer.
> ---
> Changes in v2 (Julia):
> - Use a literal function-name disjunction instead of a regex identifier,
> enabling spatch's file pre-filter optimization.
> - In "patch" mode, drop braces when only one statement remains, and
> delete the whole "if" when the message was the only (braceless)
> statement.
> - Drop two never-observed condition variants (IS_ERR(clk) == 1, ret !=
> 0); keep the one with real precedent (ret < 0).
> - Link to v1: https://patch.msgid.link/20260802-cocci-clk-register-v1-1-df68afcb1eef@gurudas.dev
> ---
> MAINTAINERS | 5 +
> scripts/coccinelle/api/clk_register.cocci | 153 ++++++++++++++++++++++++++++++
> 2 files changed, 158 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 716acfc3d7c1..26788ccbf98c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6363,6 +6363,11 @@ L: linux-clk@vger.kernel.org
> S: Maintained
> F: include/linux/clk.h
>
> +CLK_REGISTER() COCCINELLE CHECK
> +M: Guru Das Srinagesh <linux@gurudas.dev>
> +S: Maintained
> +F: scripts/coccinelle/api/clk_register.cocci
> +
I don't think there needs to be a specific maintainer for this file.
> CLOCKSOURCE, CLOCKEVENT DRIVERS
> M: Daniel Lezcano <daniel.lezcano@kernel.org>
> M: Thomas Gleixner <tglx@kernel.org>
> diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
> new file mode 100644
> index 000000000000..86a8be92f2a2
> --- /dev/null
> +++ b/scripts/coccinelle/api/clk_register.cocci
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/// Remove error messages after clk registration failures, because
> +/// clk_register(), clk_hw_register(), and their variants already log
> +/// an error when they fail. See commit 12a0fd23e870 ("clk: Print an
> +/// error when clk registration fails").
> +//
> +// Confidence: Medium
> +// Options: --include-headers
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +@depends on context@
> +expression clk;
> +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> +@@
> +
> +clk = \(clk_register\|devm_clk_register\)(...);
> +if ( IS_ERR(clk) )
> +{
> +...
> +*voidfn(...);
> +...
> +}
This will only match an if with a then branch, no else branch. There is
actually one case in the kernel where there is a relevant if with an else
branch, so you may as well extend this to match that cas too. If you just
add else S, where S is a statement metavariable that is not used
elsewhere, then it will match either the case with an else or the case
with a then branch only.
Likewise for the other rules.
[...]
> +@script:python depends on report@
> +p1 << r1.p1;
> +@@
> +
> +msg = "line %s is redundant because clk_register() already prints an error on failure" % (p1[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p2 << r2.p2;
> +@@
> +
> +cocci.print_main(p2)
This code is not correct. print_main seens a string as the first argument
and a position (which you have) as the second argument. You can make a
string in the same was as in the report rules.
The same is true for the other org mode rule. If you don't have org mode,
you can still check that the code works by testing the semantic patch with
-D org. It's just a text format.
julia
> +
> +@script:python depends on report@
> +p2 << r2.p2;
> +@@
> +
> +msg = "line %s is redundant because clk_hw_register() already prints an error on failure" % (p2[0].line)
> +coccilib.report.print_report(p2[0], msg)
>
> ---
> base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
> change-id: 20260802-cocci-clk-register-951d94251af4
>
> Best regards,
> --
> Guru Das Srinagesh <linux@gurudas.dev>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] coccinelle: Detect clk_register() anti-pattern
2026-08-09 20:00 ` Julia Lawall
@ 2026-08-10 6:30 ` Guru Das Srinagesh
2026-08-10 8:03 ` Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Guru Das Srinagesh @ 2026-08-10 6:30 UTC (permalink / raw)
To: Julia Lawall
Cc: Nicolas Palix, Michael Turquette, Stephen Boyd, linux-kernel,
cocci, Brian Masney, linux-clk
On Sun, Aug 09, 2026 at 10:00:22PM +0200, Julia Lawall wrote:
>
>
> On Mon, 3 Aug 2026, Guru Das Srinagesh wrote:
>
> > Enforce commit 12a0fd23e870 ("clk: Print an error when clk registration
> > fails"): clk_register(), clk_hw_register(), and their devm_/of_ variants
> > log their own error on failure, so driver-side error prints after these
> > calls are redundant.
> >
> > Two independent match families, one per return-value convention:
> > pointer return checked via IS_ERR() (clk_register()/devm_clk_register()),
> > and int return checked via a nonzero value (clk_hw_register()/
> > devm_clk_hw_register()/of_clk_hw_register()).
> >
> > In "patch" mode, removing the redundant message also collapses the
> > enclosing braces when only one statement remains, and deletes the whole
> > "if" when the message was already the only (braceless) statement.
> >
> > Assisted-by: Claude:claude-sonnet-5 coccinelle
> > Signed-off-by: Guru Das Srinagesh <linux@gurudas.dev>
> > ---
> > Add a Coccinelle semantic patch enforcing commit 12a0fd23e870 ("clk:
> > Print an error when clk registration fails"): flags, and in "patch"
> > mode removes, driver-side error prints that are now redundant after
> > clk_register()/clk_hw_register() and their devm_/of_ variants.
> >
> > Two independent match families, one per return-value convention.
> >
> > Pointer return, IS_ERR()-checked (clk_register()/devm_clk_register()),
> > e.g. drivers/clk/clk-xgene.c:152-157:
> >
> > clk = clk_register(dev, &apmclk->hw);
> > if (IS_ERR(clk)) {
> > - pr_err("%s: could not register clk %s\n", __func__, name);
> > kfree(apmclk);
> > return NULL;
> > }
> >
> > Int return, nonzero-checked (clk_hw_register()/devm_clk_hw_register()/
> > of_clk_hw_register()), e.g. drivers/clk/meson/meson-clkc-utils.c:49-54:
> >
> > ret = devm_clk_hw_register(dev, hw);
> > - if (ret) {
> > - dev_err(dev, "registering %s clock failed\n",
> > - hw->init->name);
> > + if (ret)
> > return ret;
> > - }
> >
> > Already-braceless single-statement case: the whole "if" is deleted
> > instead of just the message, e.g. drivers/clk/ux500/clk-sysctrl.c:171-175:
> >
> > clk_reg = devm_clk_register(clk->dev, &clk->hw);
> > - if (IS_ERR(clk_reg))
> > - dev_err(dev, "clk_sysctrl: clk_register failed\n");
> >
> > return clk_reg;
> >
> > Testing:
> > - Baseline: coccinelle 1.3.1, the Torvalds tree at v7.2-rc5.
> > - "make coccicheck COCCI=<path> MODE=report M=drivers/clk" produced 73
> > hits and verified to have zero false positives.
> > - "MODE=patch" verified separately on scratch copies of affected files
> > to confirm minimal, correct diffs.
> >
> > checkpatch flagged that this new file needs a MAINTAINERS entry, and I'd
> > like to maintain it, so this adds a standalone entry rather than leaving
> > the file uncovered. There's no direct precedent for an individual .cocci
> > file getting its own entry - the only other named .cocci file in
> > MAINTAINERS, scripts/coccinelle/api/string_choices.cocci, was added to
> > the existing GENERIC STRING LIBRARY entry by that subsystem's
> > maintainer, not as a new one. Happy to fold this into COMMON CLK
> > FRAMEWORK or drop it entirely depending on what the maintainers prefer.
> > ---
> > Changes in v2 (Julia):
> > - Use a literal function-name disjunction instead of a regex identifier,
> > enabling spatch's file pre-filter optimization.
> > - In "patch" mode, drop braces when only one statement remains, and
> > delete the whole "if" when the message was the only (braceless)
> > statement.
> > - Drop two never-observed condition variants (IS_ERR(clk) == 1, ret !=
> > 0); keep the one with real precedent (ret < 0).
> > - Link to v1: https://patch.msgid.link/20260802-cocci-clk-register-v1-1-df68afcb1eef@gurudas.dev
> > ---
> > MAINTAINERS | 5 +
> > scripts/coccinelle/api/clk_register.cocci | 153 ++++++++++++++++++++++++++++++
> > 2 files changed, 158 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 716acfc3d7c1..26788ccbf98c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6363,6 +6363,11 @@ L: linux-clk@vger.kernel.org
> > S: Maintained
> > F: include/linux/clk.h
> >
> > +CLK_REGISTER() COCCINELLE CHECK
> > +M: Guru Das Srinagesh <linux@gurudas.dev>
> > +S: Maintained
> > +F: scripts/coccinelle/api/clk_register.cocci
> > +
>
> I don't think there needs to be a specific maintainer for this file.
Removed.
>
> > CLOCKSOURCE, CLOCKEVENT DRIVERS
> > M: Daniel Lezcano <daniel.lezcano@kernel.org>
> > M: Thomas Gleixner <tglx@kernel.org>
> > diff --git a/scripts/coccinelle/api/clk_register.cocci b/scripts/coccinelle/api/clk_register.cocci
> > new file mode 100644
> > index 000000000000..86a8be92f2a2
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/clk_register.cocci
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/// Remove error messages after clk registration failures, because
> > +/// clk_register(), clk_hw_register(), and their variants already log
> > +/// an error when they fail. See commit 12a0fd23e870 ("clk: Print an
> > +/// error when clk registration fails").
> > +//
> > +// Confidence: Medium
> > +// Options: --include-headers
> > +
> > +virtual patch
> > +virtual context
> > +virtual org
> > +virtual report
> > +
> > +@depends on context@
> > +expression clk;
> > +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> > +@@
> > +
> > +clk = \(clk_register\|devm_clk_register\)(...);
> > +if ( IS_ERR(clk) )
> > +{
> > +...
> > +*voidfn(...);
> > +...
> > +}
>
> This will only match an if with a then branch, no else branch. There is
> actually one case in the kernel where there is a relevant if with an else
> branch, so you may as well extend this to match that cas too. If you just
> add else S, where S is a statement metavariable that is not used
> elsewhere, then it will match either the case with an else or the case
> with a then branch only.
>
> Likewise for the other rules.
Done, thank you for catching the edge case which I'm guessing is
drivers/media/platform/microchip/microchip-isc-clk.c.
>
> [...]
>
> > +@script:python depends on report@
> > +p1 << r1.p1;
> > +@@
> > +
> > +msg = "line %s is redundant because clk_register() already prints an error on failure" % (p1[0].line)
> > +coccilib.report.print_report(p1[0], msg)
> > +
> > +@script:python depends on org@
> > +p2 << r2.p2;
> > +@@
> > +
> > +cocci.print_main(p2)
>
> This code is not correct. print_main seens a string as the first argument
> and a position (which you have) as the second argument. You can make a
> string in the same was as in the report rules.
>
> The same is true for the other org mode rule. If you don't have org mode,
> you can still check that the code works by testing the semantic patch with
> -D org. It's just a text format.
Done, sorry for the oversight.
>
> julia
>
> > +
> > +@script:python depends on report@
> > +p2 << r2.p2;
> > +@@
> > +
> > +msg = "line %s is redundant because clk_hw_register() already prints an error on failure" % (p2[0].line)
> > +coccilib.report.print_report(p2[0], msg)
> >
> > ---
> > base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
> > change-id: 20260802-cocci-clk-register-951d94251af4
> >
> > Best regards,
> > --
> > Guru Das Srinagesh <linux@gurudas.dev>
> >
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] coccinelle: Detect clk_register() anti-pattern
2026-08-10 6:30 ` Guru Das Srinagesh
@ 2026-08-10 8:03 ` Julia Lawall
0 siblings, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2026-08-10 8:03 UTC (permalink / raw)
To: Guru Das Srinagesh
Cc: Nicolas Palix, Michael Turquette, Stephen Boyd, linux-kernel,
cocci, Brian Masney, linux-clk
> > > +@depends on context@
> > > +expression clk;
> > > +identifier voidfn =~ "^(dev_err|dev_warn|pr_err|pr_warn)$";
> > > +@@
> > > +
> > > +clk = \(clk_register\|devm_clk_register\)(...);
> > > +if ( IS_ERR(clk) )
> > > +{
> > > +...
> > > +*voidfn(...);
> > > +...
> > > +}
> >
> > This will only match an if with a then branch, no else branch. There is
> > actually one case in the kernel where there is a relevant if with an else
> > branch, so you may as well extend this to match that cas too. If you just
> > add else S, where S is a statement metavariable that is not used
> > elsewhere, then it will match either the case with an else or the case
> > with a then branch only.
> >
> > Likewise for the other rules.
>
> Done, thank you for catching the edge case which I'm guessing is
> drivers/media/platform/microchip/microchip-isc-clk.c.
Yes, that's the one.
julia
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-10 8:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04 6:20 [PATCH v2] coccinelle: Detect clk_register() anti-pattern Guru Das Srinagesh
2026-08-09 20:00 ` Julia Lawall
2026-08-10 6:30 ` Guru Das Srinagesh
2026-08-10 8:03 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox