22 Aug, 2012

1 commit

  • Commit b13edf7ff2dd ("checkpatch: add checks for do {} while (0) macro
    misuses") added a test that is overly simplistic for single statement
    macros.

    Macros that start with control tests should be enclosed in a do {} while
    (0) loop.

    Add the necessary control tests to the check.

    Signed-off-by: Joe Perches
    Acked-by: Andy Whitcroft
    Tested-by: Franz Schrober
    Cc: Stephen Rothwell
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

31 Jul, 2012

5 commits


01 Jun, 2012

2 commits

  • Suggest the shorter pr_ instead of printk(KERN_.

    Prefer to use pr_ over bare printks.
    Prefer to use pr_warn over pr_warning.

    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Cc: Theodore Ts'o
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Requires --strict option during invocation:
    ~/linux$ scripts/checkpatch --strict foo.patch

    This tests for a bad habits of mine like this:

    return 0 ;

    Note that it does allow a special case of a bare semicolon
    for empty loops:

    while (foo())
    ;

    Signed-off-by: Eric Nelson
    Cc: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Eric Nelson
     

17 Apr, 2012

1 commit


24 Mar, 2012

12 commits

  • 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

    Signed-off-by: Josh Triplett
    Acked-by: Joe Perches
    Cc: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Josh Triplett
     
  • Add blank lines between a few tests, remove an extraneous one.

    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Using yield() is generally wrong. Warn on its use.

    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Cc: Peter Zijlstra
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Add some more subjective --strict tests.

    Add a test for block comments that start with a blank line followed only
    by a line with just the comment block initiator. Prefer a blank line
    followed by /* comment...

    Add a test for unnecessary spaces after a cast.

    Add a test for symmetric uses of braces in if/else blocks.
    If one branch needs braces, then all branches should use braces.

    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Add [] to a type extensions. Fixes false positives on:

    .attrs = (struct attribute *[]) {

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • With any very high precedence operator it is not necessary to enforce
    additional parentheses around simple negated expressions. This prevents
    us requesting further perentheses around the following:

    #define PMEM_IS_FREE(id, index) !(pmem[id].bitmap[index].allocated)

    For now add logical and bitwise not and unary minus.

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Adjacent strings indicate concatentation, therefore look at identifiers
    directly adjacent to literal strings as strings too. This allows us to
    better detect the form below and accept it as a simple constant:

    #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Handle the [ A ... B ] form deeper into a definition, for example:

    static const unsigned char pci_irq_swizzle[2][PCI_MAX_DEVICES] = {
    {0, 0, 0, 0, 0, 0, 0, 27, 27, [9 ... PCI_MAX_DEVICES - 1] = 0 },
    {0, 0, 0, 0, 0, 0, 0, 29, 29, [9 ... PCI_MAX_DEVICES - 1] = 0 },
    };

    Reported-by: Marek Vasut
    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Fix checkpatch.pl when both -q and --ignore are given and prevents it from
    printing a

    NOTE: Ignored message types: blah

    messages.

    E.g., if I use -q --ignore PREFER_PACKED,PREFER_ALIGNED, i see:

    NOTE: Ignored message types: PREFER_ALIGNED PREFER_PACKED

    It makes no sense to print this when -q is given.

    Signed-off-by: Artem Bityutskiy
    Cc: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Artem Bityutskiy
     
  • Argument alignment across multiple lines should match the open
    parenthesis.

    Logical continuations should be at the end of the previous line, not the
    start of a new line.

    These are not required by CodingStyle so make the tests active only when
    using --strict.

    Improved by some examples from Bruce Allen.

    Signed-off-by: Joe Perches
    Cc: "Bruce W. Allen"
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • It's equivalent to __printf, so prefer __scanf.

    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

08 Feb, 2012

1 commit

  • Overly indented code should be refactored.

    Suggest refactoring excessive indentation of of
    if/else/for/do/while/switch statements.

    For example:

    $ cat t.c
    #include
    #include

    int main(int argc, char **argv)
    {

    if (1)
    if (2)
    if (3)
    if (4)
    if (5)
    if (6)
    if (7)
    if (8)
    ;
    return 0;
    }

    $ ./scripts/checkpatch.pl -f t.c
    WARNING: Too many leading tabs - consider code refactoring
    #12: FILE: t.c:12:
    + if (6)

    WARNING: Too many leading tabs - consider code refactoring
    #13: FILE: t.c:13:
    + if (7)

    WARNING: Too many leading tabs - consider code refactoring
    #14: FILE: t.c:14:
    + if (8)

    total: 0 errors, 3 warnings, 17 lines checked

    t.c has style problems, please review.

    If any of these errors are false positives, please report
    them to the maintainer, see CHECKPATCH in MAINTAINERS.

    Signed-off-by: Joe Perches
    Signed-off-by: Linus Torvalds

    Joe Perches
     

11 Jan, 2012

15 commits

  • Fix up type and cast spacing checks such that all occurences on a line are
    examined and reported. For example the line below has a valid cast and a
    bad type, but currently we check the cast first which is good and stop:

    u16* bar = (u16 *)baz;

    We will also only report one of the errors in this example:

    u16* bar = (u16*)bad;

    Move to iterating across all casts and all types, reporting any failure.

    [akpm@linux-foundation.org: coding-style fixes]
    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • typeof may have various more complex forms as its arguement, not just an
    identifier. For now allow us to leak to the first close perenthesis ')'.

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Ensure the cast type is unique in the context parser, we do not want them
    to detect as a comma ','.

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • We are incorrectly matching square brackets '[' and ']' leading to false
    positives on more complex functions as below:

    return (dt3155_fbuffer[m]->ready_head -
    dt3155_fbuffer[m]->ready_len +
    dt3155_fbuffer[m]->nbuffers)%
    (dt3155_fbuffer[m]->nbuffers);

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • It is common to stub out a function as below, this is triggering a complex
    macro format incorrectly. Sort this out:

    #define cma_early_regions_reserve(reserve) do { } while (0)

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • The following fragment defeats the DEVICE_ATTR style handing, check for
    and ignore the close brace '}' in this context:

    int foo()
    {
    }
    DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
    ata_scsi_lpm_show, ata_scsi_lpm_put);
    EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • The intent of this check is to catch the options which the user will see
    and ensure they are properly described. It is also common for internal
    only options to have a brief description. Allow this form.

    Reported-by: Steven Rostedt
    Tested-by: Steven Rostedt
    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • In the middle of a long definition or similar, there is no possibility of
    finding a smaller sub-statement. Optimise this case by skipping statement
    aquirey where there are no starts of statement (open brace '{' or
    semi-colon ';'). We are likely to scan slightly more than needed still
    but this is safest.

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Inserting a # into the modifiers list will incorrectly add the null string
    to the modifiers list, leading to an infinite loop. As neither of these
    is a valid modifier form simply ignore them.

    Signed-off-by: Andy Whitcroft
    Reported-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Improve the checking of arguments to memset and min/max tests.

    Move the checking of min/max to statement blocks instead of single line.
    Change $Constant to allow any case type 0x initiator and trailing ul
    specifier. Add $FuncArg type as any function argument with or without a
    cast. Print the whole statement when showing memset or min/max messages.
    Improve the memset with 0 as 3rd argument error message.

    There are still weaknesses in the $FuncArg and $Constant code as arbitrary
    parentheses and negative signs are not generically supported.

    [akpm@linux-foundation.org: fix per Andy]
    Signed-off-by: Joe Perches
    Acked-by: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Move the memset checks over to work against the statement. Also add
    checks for 0 and 1 used as lengths. Generally these indicate badly
    ordered parameters.

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • When looking for a statement we currently run on through preprocessor
    commands. This means that a header file with just definitions is parsed
    over and over again combining all of the lines from the current line to
    the end of file leading to severe performance issues.

    Fix up context accumulation to track preprocessor commands and stop when
    reaching the end of them. At the same time vastly simplify the #define
    handling.

    Signed-off-by: Andy Whitcroft
    Cc: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andy Whitcroft
     
  • Add a warn for not using __printf.

    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • email header lines can look like signature tags. It's valid to have
    multiple email recipients on a single line but not valid to have multiple
    signatures on a single line.

    Validate signatures only when not in the email headers.

    Clear the $in_commit_log flag when the patch filename appears.

    Add '-' to the valid chars in a message header for headers
    like "Message-Id:" and "In-Reply-To:".

    Signed-off-by: Joe Perches
    Reported-by: Julia Lawall
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

07 Nov, 2011

1 commit


01 Nov, 2011

2 commits