14 Oct, 2014

11 commits

  • Warn on probable misuses of logging functions with KERN_
    like pr_err(KERN_ERR "foo\n");

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

    Joe Perches
     
  • Add an exception to the return before else warning when the line
    following it is also a return like:

    if (foo)
    return bar;
    else
    return baz;

    This form of a test then return is at least as readable as

    if (foo)
    return bar;
    return baz;

    so don't emit a warning on the first form.

    Signed-off-by: Joe Perches
    Reported-by: Al Viro
    Cc: Elshad Mustafayev
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Check for misspellings, based on Debian's lintian list. Several false
    positives were removed, and several additional words added that were
    common in the kernel:

    backword backwords
    invalide valide
    recieves
    singed unsinged

    While going back and fixing existing spelling mistakes isn't a high
    priority, it'd be nice to try to catch them before they hit the tree.

    In the 13830 commits between 3.15 and 3.16, the script would have noticed
    560 spelling mistakes. The top 25 are shown here:

    $ git log --pretty=oneline v3.15..v3.16 | wc -l
    13830
    $ git log --format='%H' v3.15..v3.16 | \
    while read commit ; do \
    echo "commit $commit" ; \
    git log --format=email --stat -p -1 $commit | \
    ./scripts/checkpatch.pl --types=typo_spelling --no-summary - ; \
    done | tee spell_v3.15..v3.16.txt | grep "may be misspelled" | \
    awk '{print $2}' | tr A-Z a-z | sort | uniq -c | sort -rn
    21 'seperate'
    17 'endianess'
    15 'sucess'
    13 'noticable'
    11 'occured'
    11 'accomodate'
    10 'interrup'
    9 'prefered'
    8 'unecessary'
    8 'explicitely'
    7 'supress'
    7 'overriden'
    7 'immediatly'
    7 'funtion'
    7 'defult'
    7 'childs'
    6 'succesful'
    6 'splitted'
    6 'specifc'
    6 'reseting'
    6 'recieve'
    6 'changable'
    5 'tmis'
    5 'singed'
    5 'preceeding'

    Thanks to Joe Perches for rewrites, suggestions, additional misspelling
    entries, and testing.

    Signed-off-by: Kees Cook
    Acked-by: Joe Perches
    Cc: Masanari Iida
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Kees Cook
     
  • Macros with flow control statements (goto and return) are not very nice to
    read as any flow movement is unexpected.

    Try to highlight them and emit a warning on their definition.

    Avoid warning on macros that use argument concatenation as those macros
    commonly create another function where the concatenation is used in the
    function name definition like:

    #define FOO_FUNC(name, rtn_type) \
    rtn_type func##name(arg1, ...) \
    { \
    rtn_type rtn; \
    [code...] \
    return rtn; \
    }

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

    Joe Perches
     
  • There's a useless "+" use that needs to be removed as perl 5.20 emits a
    "Useless use of greediness modifier '+'" message each time it's hit.

    Signed-off-by: Joe Perches
    Reported-by: Greg KH
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Using a space between concatenated string elements is easier for a human
    to read.

    ie:
    "String"FOO"bar"

    is easier to read as:

    "String" FOO "bar"

    So suggest this style with a --strict command line option.

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

    Joe Perches
     
  • This script is used by many other projects, and in some of them the
    requirement of at least 4 line long description for all Kconfig items is
    excessive. This patch adds a command line option to control the required
    minimum length.

    Tested running this script over a patch including a two line config
    description. The script generated a warning when invoked as is, and did
    not generate it when invoked with --min-conf-desc-length=2.

    Signed-off-by: Vadim Bendebury
    Acked-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Vadim Bendebury
     
  • When run on *.dtsi or *.dts files, the whitespace checks were skipped,
    while they are valid for DTS files. Hence stop skipping them.

    I ran checkpatch on all in-tree DTS files, and didn't notice any error or
    warning messages that are inappropriate for DTS files.

    Signed-off-by: Geert Uytterhoeven
    Acked-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Geert Uytterhoeven
     
  • Several architectures (e.g. x86, MIPS, Blackfin) have asm/reboot.h and
    asm/time.h header files, which are not included in linux/reboot.h and
    linux/time.h headers. This lead to generation of false positive errors.

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

    Sergey Ryazanov
     
  • An unnecessary --fix debugging left-over is removed.

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

    Joe Perches
     
  • The plural of parenthesis is parentheses.

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

    Andrew Morton
     

11 Sep, 2014

1 commit

  • The general form for commit id and description is

    'Commit ("commit description/subject line")'

    but commit logs often have relatively long commit ids and the commit
    description emds on the next line like:

    Some explanation as to why commit
    ("commit foo description/subject line") is improved.

    Allow this form.

    Signed-off-by: Joe Perches
    Suggested-by: Joe Lawrence
    Tested-by: Joe Lawrence
    Suggested-by: Geert Uytterhoeven
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

30 Aug, 2014

1 commit


07 Aug, 2014

27 commits

  • Using uninitialized_var reports a false positive for "Missing blank line
    after declarations".

    Fix it by adding uninitialized_var to the $declaration_macros exceptions
    list.

    Move the macro list after $Type is declared.

    Add optional prefixes to DECLARE_ and DEFINE_
    macro declarations to allow forms like:
    MLX4_DECLARE_DOORBELL_LOCK

    Signed-off-by: Joe Perches
    Reported-by: Dotan Barak
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Checkpatch already complains when people break up quoted strings but
    it's still pretty common. One mistake that people often make is they
    leave out the space character between the two strings.

    This check adds around 450 new warnings and has a low rate of false
    positives.

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

    Dan Carpenter
     
  • Commit 89da401f6cff ("checkpatch: improve "no space after cast" test")
    in -next improved the cast test for non pointer types, but also
    introduced false positives for some types of static inlines.

    Add a test for an open brace to the exclusions to avoid these false
    positives.

    Signed-off-by: Joe Perches
    Reported-by: Hartley Sweeten
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Using --file mode can give false positives with MISSING_BREAK
    fall-through warnings on simple but long multiple consecutive case
    statements.

    Look for all lines before a case statement for a switch or a statement
    when using --file mode.

    Fix a misspelling of preceded while there.

    Signed-off-by: Joe Perches
    Reported-by: Lee Jones
    Acked-by: Lee Jones
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • c90 section "6.7.2 Type Specifiers" says:
    "type specifiers may occur in any order"

    That means that:
    short int is the same as int short
    unsigned short int is the same as int unsigned short
    etc...

    checkpatch currently parses only a subset of these allowed types.

    For instance: "unsigned short" and "signed short" are found by
    checkpatch as a specific type, but none of the or "int short" or "int
    signed short" variants are found.

    Add another table for the "kernel style misordered" variants.

    Add this misordered table to the findable types.

    Warn when the misordered style is used.

    This improves the "Missing a blank line after declarations" test as it
    depends on the correct parsing of the $Declare variable which looks for
    "$Type $Ident;" (ie: declarations like "int foo;").

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

    Joe Perches
     
  • Current generic types are unsigned or unspecified. Add signed to the
    types.

    Reorder the types to find the longest match first.

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

    Joe Perches
     
  • short int is one of the 6.7.2 c90 types.
    Find it appropriately.

    This fixes a defect in checkpatch where it suggests that a line break
    after declaration is required using an input like:

    int foo;
    short int bar;

    Without this change, it warns on the short int line.

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

    Joe Perches
     
  • All the various for_each loop macros were not tested for trailing brace
    on the following lines and for bad indentation.

    Add them.

    Signed-off-by: Joe Perches
    Reported-by: Greg KH
    Cc: Andy Whitcroft
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Add --fix corrections for ELSE_AFTER_BRACE and WHILE_AFTER_BRACE
    misuses.

    if (x) {
    ...
    }
    else {
    ...
    }

    is corrected to

    if (x) {
    ...
    } else {
    ...
    }

    and

    do {
    ...
    }
    while (x);

    is corrected to

    do {
    ...
    } while (x);

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

    Joe Perches
     
  • Style misuses of these types are corrected:

    typedef struct foo
    {
    int bar;
    };

    int foo(int bar) { return bar+1;
    }

    int foo(int bar) {
    return bar+1;
    }

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

    Joe Perches
     
  • I copied the which subroutine from get_maintainer.pl.

    Unfortunately, get_maintainer uses a 4 space indentation so use the
    proper tab indentation instead.

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

    Joe Perches
     
  • Neaten the uses of patch/file line insertions or deletions. Hide the
    mechanism used.

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

    Joe Perches
     
  • This can be valuable to insert or delete blank lines as well as fix
    misplaced brace or else uses.

    Store indexes of lines to be added/deleted and the new lines.

    When creating the --fix file, insert or delete the appropriate lines and
    update the patch range information.

    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Cc: Dan Carpenter
    Cc: Josh Triplett
    Cc: Greg Kroah-Hartman
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Make the fix code a bit easier to read.

    This should also start to allow an easier mechanism to insert/delete
    lines eventually too.

    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Cc: Dan Carpenter
    Cc: Josh Triplett
    Cc: Greg Kroah-Hartman
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Using break; after a goto or return is unnecessary so emit a warning
    when the break is at the same indent level.

    So this emits a warning on:

    switch (foo) {
    case 1:
    goto err;
    break;
    }

    but not on:

    switch (foo) {
    case 1:
    if (bar())
    goto err;
    break;
    }

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

    Joe Perches
     
  • Whenever files are added, moved, or deleted, the MAINTAINERS file
    patterns can be out of sync or outdated.

    To try to keep MAINTAINERS more up-to-date, add a one-time warning
    whenever a patch does any of those.

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

    Joe Perches
     
  • Commit logs have various forms of commit id references.

    Try to standardize on a 12 character long lower case commit id along
    with a description of parentheses and the quoted subject line.

    ie: commit 0123456789ab ("commit description")

    If git and a git tree exists, look up the commit id and emit the
    appropriate line as part of the message.

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

    Joe Perches
     
  • Avoid matching allocs that appear to be known small multiplications of a
    sizeof with a constant because gcc as of 4.8 cannot optimize the code in
    a calloc() exactly the same way as an alloc().

    Look for numeric constants or what appear to be upper case only macro
    #defines.

    Signed-off-by: Joe Perches
    Reported-by: Theodore Ts'o
    Original-patch-by: Fabian Frederick
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • This --strict test previously worked only for what appeared to be cast
    to pointer types.

    Make it work for all casts.

    Also, there's no reason to show the previous line for this type of
    message, so don't.

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

    Joe Perches
     
  • checkpatch's $Type variable does not match declarations of multiple
    const * types.

    This can produce false positives for things like:

    $ ./scripts/checkpatch.pl -f drivers/staging/comedi/comedidev.h
    WARNING: Missing a blank line after declarations
    #60: FILE: drivers/staging/comedi/comedidev.h:60:
    + const struct comedi_lrange *range_table;
    + const struct comedi_lrange *const *range_table_list;

    Fix the $Type variable to support matching multiple "* const" uses.

    Signed-off-by: Joe Perches
    Reported-by: Hartley Sweeten
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Parentheses around &(foo->bar) and *(foo->bar) are unnecessary. Emit a
    --strict only message on these uses.

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

    Joe Perches
     
  • Editing Kconfig dependencies can emit unnecessary messages about missing
    or too short help entries.

    Only emit the message when adding help sections to Kconfig files.

    Signed-off-by: Joe Perches
    Reported-by: Jean Delvare
    Tested-by: Jean Delvare
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Make it consistent with the other missing or multiple blank line tests.

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

    Joe Perches
     
  • Multiple consecutive blank lines waste screen space. Emit a --strict
    only message with these blank lines.

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

    Joe Perches
     
  • Add a --strict test asking for a blank line after
    function/struct/union/enum declarations.

    Allow exceptions for several attributes and macro uses.

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

    Joe Perches
     
  • This might help a kernel hacker think twice before blindly adding a
    newline.

    Signed-off-by: Rasmus Villemoes
    Acked-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Rasmus Villemoes
     
  • There are some patches created by git format-patch that when scanned by
    checkpatch report errors on lines like

    To: address.tld

    This is a checkpatch false positive.

    Improve the logic a bit to ignore folded email headers to avoid emitting
    these messages.

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

    Joe Perches