checkpatch: Check for quoted strings broken across lines

checkpatch already makes an exception to the 80-column rule for quoted strings, and Documentation/CodingStyle recommends not splitting quoted strings across lines, because it breaks the ability to grep for the string. Rather than just permitting this, actively warn about quoted strings split across lines.

Test case:

void context(void)

{
	struct { unsigned magic; const char *strdata; } foo[] = {
		{ 42, "these strings"
		      "do not produce warnings" },
		{ 256, "though perhaps"
		       "they should" },
	};
	pr_err("this string"
	       " should produce a warning\n");
	pr_err("this multi-line string\n"
	       "should not produce a warning\n");
	asm ("this asm\n\t"
	     "should not produce a warning");
}

Results of checkpatch on that test case:

WARNING: quoted string split across lines
+ " should produce a warning\n");

total: 0 errors, 1 warnings, 15 lines checked

---

Resending now that the merge window has opened.

 scripts/checkpatch.pl |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

index e3bfcbe..01a5e4b 100755
@@ -1737,6 +1737,21 @@ sub process {
 			     "line over 80 characters\n" . $herecurr);
 		}
 
+# Check for user-visible strings broken across lines, which breaks the ability
+# to grep for the string.  Limited to strings used as parameters (those
+# following an open parenthesis), which almost completely eliminates false
+# positives, as well as warning only once per parameter rather than once per
+# line of the string.  Make an exception when the previous string ends in a
+# newline (multiple lines in one string constant) or \n\t (common in inline
+# assembly to indent the instruction on the following line).
+		if ($line =~ /^\+\s*"/ &&
+		    $prevline =~ /"\s*$/ &&
+		    $prevline =~ /\(/ &&
+		    $prevrawline !~ /\\n(?:\\t)*"\s*$/) {
+			WARN("SPLIT_STRING",
+			     "quoted string split across lines\n" . $hereprev);
+		}
+
 # check for spaces before a quoted newline
 		if ($rawline =~ /^.*\".*\s\\n/) {
 			WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",

1.7.9

Hi Josh.

I don't recall seeing your patch before, but I submitted a similar one that I believe Andrew has in his tree.

https://lkml.org/lkml/2012/3/2/24

You reviewed my patch at the time and provided feedback, and I'd produced a revised version based on that feedback.

The heuristics in the patch I submitted almost completely eliminate false positives, which makes my patch suitable for use without --strict. Having a --strict version which flags *all* wrapped strings seems potentially reasonable as well (though you'll still want the heuristic that ignores strings ending in \n or \n\t), but I'd still like to get my version included with the heuristics that make it suitable as a default.

Among other things, the version you submitted to Andrew will flag multi-instruction __asm__ directives, multi-line strings printed with a single printk, and arrays of arbitrary data expressed as strings.

- Josh Triplett

duh.

I heard short term memory fades as you get age,
but I might not remember that in awhile.

Yup, this patch is better than the one I submitted.

One improvement might be to check for a line continuation \
on $prevline and still produce a warning in that case.

cheers, Joe

OK. Andrew, could you substitute my patch for Joe's in your tree?

I'd suggest doing that one as a separate check, "unnecessary line continuation", which should flag any use of a line continuation other than with a preprocessor directive. I don't think combining that with the wrapped-string check makes sense.

- Josh Triplett

What I suggested is for a case in a #define:

#define subsystem_printk(subsystem, level, format, ...)			\
	netdev_printk(level, (subsystem)->dev, "Some partial format "	\
		      "continued on another line" format,		\
		      ##__VA_ARGS__)

where it's necessary to use line continuations but rewrapping like:

#define subsystem_printk(subsystem, level, format, ...)			\
	netdev_printk(level, (subsystem)->dev, 				\
		      "A coalesced format on a single line" format,	\

##__VA_ARGS__)

might improve it.

Anyway: