* [RFC PATCH] checkpatch: check formatting of __func__ output uses @ 2016-03-13 19:19 Joe Perches 2016-03-14 0:42 ` [RFC PATCH V2] checkpatch: Check output format style of __func__ uses Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2016-03-13 19:19 UTC (permalink / raw) To: Andrew Morton, Andy Whitcroft Cc: Dan Carpenter, Julia Lawall, kernel-janitors, linux-kernel Loggng messages that emit function names have many different forms. Right now, grep shows these mixtures of forms: 13704 "%s:" 3839 "%s " 2787 "%s()" Some of these are in macros. Perhaps it'd be better for grep and consistency to exclusively use "%s:" Unfortunately, checkpatch isn't an ideal tool to find all these uses. It seems difficult to handle the possible macro definition styles. Maybe coccinelle might be better at it, but here's a possible patch to find the non-macro definition uses. Signed-off-by: Joe Perches <joe@perches.com> --- scripts/checkpatch.pl | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 75ce6d0..727ab64 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1415,6 +1415,22 @@ sub raw_line { return $line; } +sub cooked_line { + my ($linenr, $cnt) = @_; + + my $offset = $linenr - 1; + $cnt++; + + my $line; + while ($cnt) { + $line = $lines[$offset++]; + next if (defined($line) && $line =~ /^-/); + $cnt--; + } + + return $line; +} + sub cat_vet { my ($vet) = @_; my ($res, $coded); @@ -5681,6 +5697,33 @@ sub process { } } +# check how __func__ is formatted, prefer "%s:...', __func__ + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /\b__func__\b/ && + $stat =~ /^\+\s*$logFunctions\s*\(\s*[^"]*$String\s*,\s*__func__\b/m && + (() = $stat =~ /^\+|\n\+/g) == 1 && + (() = $stat =~ /;/g) <= 1) { + my $herectx = $here . "\n"; + my $cooked_linenr = -1; + my $cooked_line = ""; + my $raw_line = ""; + my $cnt = statement_rawlines($stat); + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + if ($cooked_linenr == -1 && cooked_line($linenr, $n) =~ /$String/) { + $cooked_linenr = $linenr + $n; + $cooked_line = cooked_line($linenr, $n); + $raw_line = raw_line($linenr, $n); + } + } + my $qs = get_quoted_string($cooked_line, $raw_line); + if ($qs !~ /^"%s:/) { + WARN("FUNC_STYLE", + "Prefer using formatting style '%s:' for __func__\n" . $herectx); + } + } + # check for uses of __DATE__, __TIME__, __TIMESTAMP__ while ($line =~ /\b(__(?:DATE|TIME|TIMESTAMP)__)\b/g) { ERROR("DATE_TIME", -- 2.6.3.368.gf34be46 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH V2] checkpatch: Check output format style of __func__ uses 2016-03-13 19:19 [RFC PATCH] checkpatch: check formatting of __func__ output uses Joe Perches @ 2016-03-14 0:42 ` Joe Perches 2016-03-14 5:19 ` Julia Lawall 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2016-03-14 0:42 UTC (permalink / raw) To: Andrew Morton, Andy Whitcroft Cc: Dan Carpenter, Julia Lawall, kernel-janitors, linux-kernel Loggng messages that emit function names have many different forms. Perhaps it'd be better for logging consistency and grep ease to exclusively use "%s:" As well, function tracing logging uses are generally unnecessary given the kernel's function tracing (ftrace) capability. Right now, grep shows these mixtures of forms: 13704 "%s:" 3839 "%s " 2787 "%s()" Some of these are macros definitions of various styles. Unfortunately, given the complexity of these macro definition styles, checkpatch isn't an ideal tool to find these macros. Maybe a coccinelle script might be better suited to find and fix all the various types of uses. Add a --fix option for these logging messages with __func__. Signed-off-by: Joe Perches <joe@perches.com> --- v2: Warn on function tracing logging Add --fix option scripts/checkpatch.pl | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 75ce6d0..b695f75 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1415,6 +1415,22 @@ sub raw_line { return $line; } +sub cooked_line { + my ($linenr, $cnt) = @_; + + my $offset = $linenr - 1; + $cnt++; + + my $line; + while ($cnt) { + $line = $lines[$offset++]; + next if (defined($line) && $line =~ /^-/); + $cnt--; + } + + return $line; +} + sub cat_vet { my ($vet) = @_; my ($res, $coded); @@ -5681,6 +5697,45 @@ sub process { } } +# check how __func__ is formatted, prefer "%s:...', __func__ + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /\b__func__\b/ && + $stat =~ /^\+\s*$logFunctions\s*\(\s*[^"]*$String\s*,\s*__func__\b/m && + (() = $stat =~ /^\+|\n\+/g) == 1 && + (() = $stat =~ /;/g) <= 1) { + my $herectx = $here . "\n"; + my $cooked_linenr = -1; + my $cooked_line = ""; + my $raw_line = ""; + my $cnt = statement_rawlines($stat); + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + if ($cooked_linenr == -1 && cooked_line($linenr, $n) =~ /$String/) { + $cooked_linenr = $linenr + $n; + $cooked_line = cooked_line($linenr, $n); + $raw_line = raw_line($linenr, $n); + } + } + my $qs = get_quoted_string($cooked_line, $raw_line); + if ($qs =~ /^"\s*%s(?:[\s:\-]*|[\s:\-]*\(\s*\)\s*[\s:\-]*)?(?:enter|entering|entered|exit|exiting)?\s*\.*\s*\\n"$/i) { + if (WARN("FUNC_STYLE", + "Prefer using ftrace to logging function entry/exit\n" . $herectx) && + $cnt == 1 && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } + } elsif ($qs !~ /^"%s:/) { + if (WARN("FUNC_STYLE", + "Prefer using formatting style '%s:' for __func__\n" . $herectx) && + $fix) { + $fixed[$cooked_linenr - 1] =~ s/[:\s]*%s(?:[:\s,\-]*|[\s:\-]*\(\s*\)\s*[\s:\-]*)?//; + $fixed[$cooked_linenr - 1] =~ s/"/"%s: /; + $fixed[$cooked_linenr - 1] =~ s/"%s: \\n/"%s\\n/; + } + } + } + # check for uses of __DATE__, __TIME__, __TIMESTAMP__ while ($line =~ /\b(__(?:DATE|TIME|TIMESTAMP)__)\b/g) { ERROR("DATE_TIME", -- 2.6.3.368.gf34be46 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH V2] checkpatch: Check output format style of __func__ uses 2016-03-14 0:42 ` [RFC PATCH V2] checkpatch: Check output format style of __func__ uses Joe Perches @ 2016-03-14 5:19 ` Julia Lawall 2016-03-14 7:35 ` Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Julia Lawall @ 2016-03-14 5:19 UTC (permalink / raw) To: Joe Perches Cc: Andrew Morton, Andy Whitcroft, Dan Carpenter, Julia Lawall, kernel-janitors, linux-kernel On Sun, 13 Mar 2016, Joe Perches wrote: > Loggng messages that emit function names have many different forms. > Perhaps it'd be better for logging consistency and grep ease to > exclusively use "%s:" > > As well, function tracing logging uses are generally unnecessary given > the kernel's function tracing (ftrace) capability. > > Right now, grep shows these mixtures of forms: > > 13704 "%s:" > 3839 "%s " > 2787 "%s()" > > Some of these are macros definitions of various styles. > > Unfortunately, given the complexity of these macro definition styles, > checkpatch isn't an ideal tool to find these macros. > > Maybe a coccinelle script might be better suited to find and fix all > the various types of uses. > > Add a --fix option for these logging messages with __func__. I'm not good enough at perl to really understand this. Coudl you give an example of what it does, and of what it does not do? thanks, julia > > Signed-off-by: Joe Perches <joe@perches.com> > --- > > v2: Warn on function tracing logging > Add --fix option > > scripts/checkpatch.pl | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 75ce6d0..b695f75 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1415,6 +1415,22 @@ sub raw_line { > return $line; > } > > +sub cooked_line { > + my ($linenr, $cnt) = @_; > + > + my $offset = $linenr - 1; > + $cnt++; > + > + my $line; > + while ($cnt) { > + $line = $lines[$offset++]; > + next if (defined($line) && $line =~ /^-/); > + $cnt--; > + } > + > + return $line; > +} > + > sub cat_vet { > my ($vet) = @_; > my ($res, $coded); > @@ -5681,6 +5697,45 @@ sub process { > } > } > > +# check how __func__ is formatted, prefer "%s:...', __func__ > + if ($^V && $^V ge 5.10.0 && > + defined $stat && > + $stat =~ /\b__func__\b/ && > + $stat =~ /^\+\s*$logFunctions\s*\(\s*[^"]*$String\s*,\s*__func__\b/m && > + (() = $stat =~ /^\+|\n\+/g) == 1 && > + (() = $stat =~ /;/g) <= 1) { > + my $herectx = $here . "\n"; > + my $cooked_linenr = -1; > + my $cooked_line = ""; > + my $raw_line = ""; > + my $cnt = statement_rawlines($stat); > + for (my $n = 0; $n < $cnt; $n++) { > + $herectx .= raw_line($linenr, $n) . "\n"; > + if ($cooked_linenr == -1 && cooked_line($linenr, $n) =~ /$String/) { > + $cooked_linenr = $linenr + $n; > + $cooked_line = cooked_line($linenr, $n); > + $raw_line = raw_line($linenr, $n); > + } > + } > + my $qs = get_quoted_string($cooked_line, $raw_line); > + if ($qs =~ /^"\s*%s(?:[\s:\-]*|[\s:\-]*\(\s*\)\s*[\s:\-]*)?(?:enter|entering|entered|exit|exiting)?\s*\.*\s*\\n"$/i) { > + if (WARN("FUNC_STYLE", > + "Prefer using ftrace to logging function entry/exit\n" . $herectx) && > + $cnt == 1 && > + $fix) { > + fix_delete_line($fixlinenr, $rawline); > + } > + } elsif ($qs !~ /^"%s:/) { > + if (WARN("FUNC_STYLE", > + "Prefer using formatting style '%s:' for __func__\n" . $herectx) && > + $fix) { > + $fixed[$cooked_linenr - 1] =~ s/[:\s]*%s(?:[:\s,\-]*|[\s:\-]*\(\s*\)\s*[\s:\-]*)?//; > + $fixed[$cooked_linenr - 1] =~ s/"/"%s: /; > + $fixed[$cooked_linenr - 1] =~ s/"%s: \\n/"%s\\n/; > + } > + } > + } > + > # check for uses of __DATE__, __TIME__, __TIMESTAMP__ > while ($line =~ /\b(__(?:DATE|TIME|TIMESTAMP)__)\b/g) { > ERROR("DATE_TIME", > -- > 2.6.3.368.gf34be46 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH V2] checkpatch: Check output format style of __func__ uses 2016-03-14 5:19 ` Julia Lawall @ 2016-03-14 7:35 ` Joe Perches 2016-03-14 8:29 ` Julia Lawall 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2016-03-14 7:35 UTC (permalink / raw) To: Julia Lawall Cc: Andrew Morton, Andy Whitcroft, Dan Carpenter, kernel-janitors, linux-kernel On Mon, 2016-03-14 at 06:19 +0100, Julia Lawall wrote: > On Sun, 13 Mar 2016, Joe Perches wrote: > > Loggng messages that emit function names have many different forms. > > Perhaps it'd be better for logging consistency and grep ease to > > exclusively use "%s:" > > > > As well, function tracing logging uses are generally unnecessary given > > the kernel's function tracing (ftrace) capability. > > > > Right now, grep shows these mixtures of forms: > > > > 13704 "%s:" > > 3839 "%s " > > 2787 "%s()" > > > > Some of these are macros definitions of various styles. > > > > Unfortunately, given the complexity of these macro definition styles, > > checkpatch isn't an ideal tool to find these macros. > > > > Maybe a coccinelle script might be better suited to find and fix all > > the various types of uses. > > > > Add a --fix option for these logging messages with __func__. > > I'm not good enough at perl to really understand this. Coudl you give an > example of what it does, and of what it does not do? For instance, this could do simple conversions like: $ diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c @@ -416 +416 @@ int __init mcpm_loopback(void (*cache_disable)(void)) - pr_err("%s returned %d\n", __func__, ret); + pr_err("%s: returned %d\n", __func__, ret); But it couldn't find/convert a string concatenation: #define pch_dbg(adap, fmt, arg...) \ dev_dbg(adap->pch_adapter.dev.parent, "%s :" fmt, __func__, ##arg) cheers, Joe ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH V2] checkpatch: Check output format style of __func__ uses 2016-03-14 7:35 ` Joe Perches @ 2016-03-14 8:29 ` Julia Lawall 2016-03-14 14:07 ` Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Julia Lawall @ 2016-03-14 8:29 UTC (permalink / raw) To: Joe Perches Cc: Julia Lawall, Andrew Morton, Andy Whitcroft, Dan Carpenter, kernel-janitors, linux-kernel [-- Attachment #1: Type: TEXT/PLAIN, Size: 1721 bytes --] On Mon, 14 Mar 2016, Joe Perches wrote: > On Mon, 2016-03-14 at 06:19 +0100, Julia Lawall wrote: > > On Sun, 13 Mar 2016, Joe Perches wrote: > > > Loggng messages that emit function names have many different forms. > > > Perhaps it'd be better for logging consistency and grep ease to > > > exclusively use "%s:" > > > > > > As well, function tracing logging uses are generally unnecessary given > > > the kernel's function tracing (ftrace) capability. > > > > > > Right now, grep shows these mixtures of forms: > > > > > > 13704 "%s:" > > > 3839 "%s " > > > 2787 "%s()" > > > > > > Some of these are macros definitions of various styles. > > > > > > Unfortunately, given the complexity of these macro definition styles, > > > checkpatch isn't an ideal tool to find these macros. > > > > > > Maybe a coccinelle script might be better suited to find and fix all > > > the various types of uses. > > > > > > Add a --fix option for these logging messages with __func__. > > > > I'm not good enough at perl to really understand this. Coudl you give an > > example of what it does, and of what it does not do? > > For instance, this could do simple conversions like: > > $ diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c > @@ -416 +416 @@ int __init mcpm_loopback(void (*cache_disable)(void)) > - pr_err("%s returned %d\n", __func__, ret); > + pr_err("%s: returned %d\n", __func__, ret); > > But it couldn't find/convert a string concatenation: > > #define pch_dbg(adap, fmt, arg...) \ > dev_dbg(adap->pch_adapter.dev.parent, "%s :" fmt, __func__, ##arg) OK, are there any thoughts about what to do when __func__ is not in the first position? julia ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH V2] checkpatch: Check output format style of __func__ uses 2016-03-14 8:29 ` Julia Lawall @ 2016-03-14 14:07 ` Joe Perches 0 siblings, 0 replies; 6+ messages in thread From: Joe Perches @ 2016-03-14 14:07 UTC (permalink / raw) To: Julia Lawall Cc: Andrew Morton, Andy Whitcroft, Dan Carpenter, kernel-janitors, linux-kernel On Mon, 2016-03-14 at 09:29 +0100, Julia Lawall wrote: > On Mon, 14 Mar 2016, Joe Perches wrote: > > On Mon, 2016-03-14 at 06:19 +0100, Julia Lawall wrote: > > > On Sun, 13 Mar 2016, Joe Perches wrote: > > > > Loggng messages that emit function names have many different forms. > > > > Perhaps it'd be better for logging consistency and grep ease to > > > > exclusively use "%s:" > > > > > > > > As well, function tracing logging uses are generally unnecessary given > > > > the kernel's function tracing (ftrace) capability. > > > > > > > > Right now, grep shows these mixtures of forms: > > > > > > > > 13704 "%s:" > > > > 3839 "%s " > > > > 2787 "%s()" > > > > > > > > Some of these are macros definitions of various styles. > > > > > > > > Unfortunately, given the complexity of these macro definition styles, > > > > checkpatch isn't an ideal tool to find these macros. > > > > > > > > Maybe a coccinelle script might be better suited to find and fix all > > > > the various types of uses. > > > > > > > > Add a --fix option for these logging messages with __func__. > > > > > > I'm not good enough at perl to really understand this. Coudl you give an > > > example of what it does, and of what it does not do? > > > > For instance, this could do simple conversions like: > > > > $ diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c > > @@ -416 +416 @@ int __init mcpm_loopback(void (*cache_disable)(void)) > > - pr_err("%s returned %d\n", __func__, ret); > > + pr_err("%s: returned %d\n", __func__, ret); > > > > But it couldn't find/convert a string concatenation: > > > > #define pch_dbg(adap, fmt, arg...) \ > > dev_dbg(adap->pch_adapter.dev.parent, "%s :" fmt, __func__, ##arg) > > OK, are there any thoughts about what to do when __func__ is not in the > first position? Hard to say, Uses with any or all of __FILE__, __LINE__, or __func__ in various combinations might be standardized. A lot of those are debugging style macro and probably could be converted to dynamic_debug style uses without any mention of __FILE__, __LINE__, or __func__ at all as dynamic_debug can optionally output these. For macros like: drivers/mtd/ubi/ubi.h-#define ubi_err(ubi, fmt, ...) pr_err(UBI_NAME_STR "%d error: %s: " fmt "\n", \ drivers/mtd/ubi/ubi.h: ubi->ubi_num, __func__, ##__VA_ARGS__) it might be best to leave it alone. as that lets grep find these a bit easier using "^ubi" though code style this might be nicer on 3 lines #define ubi_err(ubi, fmt, ...) \ pr_err(UBI_NAME_STR "%d error: %s: " fmt "\n", \ ubi->ubi_num, __func__, ##__VA_ARGS__) or maybe even as a function rather than a macro if it saves code space. __func__ could then be converted to __builtin_return_address(0) for instance: https://lkml.org/lkml/2016/2/25/554 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-14 14:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-13 19:19 [RFC PATCH] checkpatch: check formatting of __func__ output uses Joe Perches 2016-03-14 0:42 ` [RFC PATCH V2] checkpatch: Check output format style of __func__ uses Joe Perches 2016-03-14 5:19 ` Julia Lawall 2016-03-14 7:35 ` Joe Perches 2016-03-14 8:29 ` Julia Lawall 2016-03-14 14:07 ` Joe Perches
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).