Messages in this thread | | | Date | Sun, 11 Sep 2022 15:21:54 +0200 | Subject | Re: [RFC PATCH] checkpatch: Check check for places where dev_err_probe() would likely be better than dev_err() | From | Christophe JAILLET <> |
| |
Le 11/09/2022 à 15:15, Christophe JAILLET a écrit : > Some functions are known to potentially return -EPROBE_DEFER. In such a > case, it is likely that dev_err_probe() is a better choice than err_err(). > > dev_err_probe(): > - is usually less verbose > - generates smaller .o files > - handles -EPROBE_DEFER so that logs are not spammed > - automatically log the error code in a human readable way > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > This patch is only a PoC to see if there is some interest in such a new > check. > The hard coded '5 lines of context' has been chosen because a typical > pattern is: > > clk = devm_clk_get(dev, "clk_lcd"); > if (IS_ERR(clk) { > dev_err(dev, "Error meesage\n"); > return PTR_ERR(clk); > }
(adding Linus Walleij)
I forgot to say that this patch is a try to address the comment from Linus Walleij at [1].
It would not help "fixing a gazillion dev_err_probe()", but it could help not having more to fix later :)
CJ
[1]: https://lore.kernel.org/all/CACRpkdZEcTD1A3tR=d4fDF89ECMDfchVPW921v6X6ARiPXHEMQ@mail.gmail.com/
> --- > scripts/checkpatch.pl | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 2737e4ced574..88365749ed2e 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2625,6 +2625,9 @@ sub process { > my $last_blank_line = 0; > my $last_coalesced_string_linenr = -1; > > + my $last_function_that_return_defer = ""; > + my $last_function_that_return_defer_linenr = 0; > + > our @report = (); > our $cnt_lines = 0; > our $cnt_error = 0; > @@ -7459,6 +7462,17 @@ sub process { > WARN("DUPLICATED_SYSCTL_CONST", > "duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr); > } > + > +# check for places where dev_err_probe() would likely be better than dev_err() > + if ($line =~ /((?:devm_)?clk_get)s*\(/) { > + $last_function_that_return_defer = $1; > + $last_function_that_return_defer_linenr = $linenr; > + } > + if ($last_function_that_return_defer_linenr >= ($linenr - 5) && > + $line =~ /dev_err[^_]/) { > + WARN("LIKELY_DEV_ERR_PROBE", > + "dev_err_probe() is likely a better choice than err_err() after a " . $last_function_that_return_defer . "() call\n" . $herecurr); > + } > } > > # If we have no input at all, then there is nothing to report on
| |