Chekpatch does not warn about some quoted string split across lines

Chekpatch does not warn about some quoted string split across lines

[Anmol Sarma]

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!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20121024/8548290f/attachment.html 

Chekpatch does not warn about some quoted string split across lines

[Jorgyano Vieira]

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.