On Wed, Oct 24, 2012 at 12:01:16PM +0530, Anmol Sarma wrote:

I've noticed that that some quoted string split across lines are not reported by checkpatch.pl. For example, running it on /drivers/staging/android/binder.c does not warn of multi-line strings at lines 512, 544, 776, 804 and several others. Does anyone know why this is happening?

Thanks!

well, checkpatch.pl is a very helpful script but it is still a buggy one. see below the part of the code (checkpatch.pl:1779) where split strings is checked and warned: ------------------------ START ------------------------------------------------- # 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); } ------------------------ END --------------------------------------------------- Now, let's analyze the conditions to trigger the warnning: condition 1] # True if the line with double quotes (we can have whitespaces before it) $line =~ /^\+\s*"/ && condition 2] # True if the previous line ends with a double quote (we can have # whitespaces after it) $prevline =~ /"\s*$/ && condition 3] # True if the previous line has a open parenthesis (we are looking for # strings as parameters only) $prevline =~ /\(/ && condition 4] # True if the previous line don't have a multiple lines in one string $prevrawline !~ /\\n(?:\\t)*"\s*$/) { WARN("SPLIT_STRING", "quoted string split across lines\n" . $hereprev); So, let's look at an example of the kind line which checkpatch.pl does not trigger the warning when it should: driver/staging/android/binder.c:544 ------------------------ SART -------------------------------------------------- binder_debug(BINDER_DEBUG_BUFFER_ALLOC, "binder: %d: add free buffer, size %zd, " "at %p\n", proc->pid, new_buffer_size, new_buffer); ------------------------ END --------------------------------------------------- When checkpatch.pl reads the line 544 (the line which it should trgger the warning) of binder.c and enters the "split string check" we have: condition 1: matches, the third line of the code snippet, it ends with a '"' condition 2: matches, the previous line ends with a double quote condition 3: NOT MATCH, the previous line does not have a open paranthesis So the reason why checkpatch.pl does not trigger the warning on these lines is: _the opening parenthesis is not on the previous line_, but on the line before the previous line, and checkpatch does not foresee this particular case.