kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
From: jorgyano@gmail.com (Jorgyano Vieira)
To: kernelnewbies@lists.kernelnewbies.org
Subject: Chekpatch does not warn about some quoted string split across lines
Date: Wed, 24 Oct 2012 11:09:28 -0200	[thread overview]
Message-ID: <20121024130928.GA18606@debian.Home> (raw)
In-Reply-To: <CAP0ktj+vpWDVdSp_MYj57jY_VEJyCuqoasSGNFxtLiZtoU6xKA@mail.gmail.com>

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.

      reply	other threads:[~2012-10-24 13:09 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-24  6:31 Chekpatch does not warn about some quoted string split across lines Anmol Sarma
2012-10-24 13:09 ` Jorgyano Vieira [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121024130928.GA18606@debian.Home \
    --to=jorgyano@gmail.com \
    --cc=kernelnewbies@lists.kernelnewbies.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).