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.
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
