12 Oct, 2016

12 commits

  • The function calls with octal permissions commonly span multiple lines.
    The current test is line oriented and fails to find some matches.

    Make the test use the $stat variable instead of the $line variable to span
    multiple lines.

    Also add a few functions to the known functions with permissions list.

    Move the SYMBOLIC_PERMS test to a separate section to find all the S_
    permissions in any form not just those that have specific function names.

    This can now find and fix permissions uses like:
    .mode = S_ | S_;

    Link: http://lkml.kernel.org/r/b51bab60530912aae4ac420119d465c5b206f19f.1475030406.git.joe@perches.com
    Signed-off-by: Joe Perches
    Tested-by: Ramiro Oliveira
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Function definitions without identifiers like
    int foo(int)
    are not preferred. Emit a warning when they occur.

    Link: http://lkml.kernel.org/r/94fe6378504745991b650f48fc92bb4648f25706.1474925354.git.joe@perches.com
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • It is possible for a multiple line macro definition to have a false positive
    report when an argument is used on a line after a continuation \.

    This line might have a leading '+' as the initial character that could be
    confused by checkpatch as an operator.

    Avoid the leading character on multiple line macro definitions.

    Link: http://lkml.kernel.org/r/60229d13399f9b6509db5a32e30d4c16951a60cd.1473836073.git.joe@perches.com
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Add a test for macro arguents that have a non-comma leading or trailing
    operator where the argument isn't parenthesized to avoid possible precedence
    issues.

    Link: http://lkml.kernel.org/r/47715508972f8d786f435e583ff881dbeee3a114.1473745855.git.joe@perches.com
    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Cc: Julia Lawall
    Cc: Dan Carpenter
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • If a macro argument is used multiple times in the macro definition, the
    macro argument may have an unexpected side-effect.

    Add a test (MACRO_ARG_REUSE) for that condition which is only
    emitted with command-line option --strict.

    Link: http://lkml.kernel.org/r/b6d67a87cafcafd15499e91780dc63b15dec0aa0.1473744906.git.joe@perches.com
    Signed-off-by: Joe Perches
    Cc: Andy Whitcroft
    Cc: Julia Lawall
    Cc: Dan Carpenter
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • An "uninitialized value" is emitted when a block comment starts on
    the same line as a statement.

    Fix this and make the test use a little fewer cpu cycles too.

    Link: http://lkml.kernel.org/r/3c9993320c2182d37f53ac540878cfef59c3f62d.1473365956.git.joe@perches.com
    Signed-off-by: Joe Perches
    Reported-by: Charlemagne Lasse
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Adding -f to the get_maintainer.pl invocation means git isn't invoked
    by get_maintainer.pl for known filenames.

    This reduces the overall time to run checkpatch.

    Link: http://lkml.kernel.org/r/22991e3a295aeb399b43af0478b6e5809106ccee.1472684066.git.joe@perches.com
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Make it easier to add new structs that should be const.

    Link: http://lkml.kernel.org/r/e5a8da43e7c11525bafbda1ca69a8323614dd942.1472664220.git.joe@perches.com
    Signed-off-by: Joe Perches
    Cc: Julia Lawall
    Cc: Kees Cook
    Cc: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • < sigh > Comment these tests out.

    These are just too enticing to people that don't verify that
    both source and dest addresses really must be __aligned(2).

    It helps make Dan Carpenter happy too.

    Link: http://lkml.kernel.org/r/dc32ec66d24647f4cdf824c8dfbbc59aa7ce7b7d.1472665676.git.joe@perches.com
    Signed-off-by: Joe Perches
    Cc: Dan Carpenter
    Cc: Greg
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Warn when block comments are not aligned on the *

    /*
    * block comment, no warning
    */

    /*
    * block comment, emit warning
    */

    Link: http://lkml.kernel.org/r/edb57bd330adfe024b95ec2a807d4aa7f0c8b112.1472261299.git.joe@perches.com
    Signed-off-by: Joe Perches
    Reported-by: Sudip Mukherjee
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • S_ uses should be avoided where octal is more intelligible.

    Linus didst say:

    : It's *much* easier to parse and understand the octal numbers, while the
    : symbolic macro names are just random line noise and hard as hell to
    : understand. You really have to think about it.
    :
    : So we should rather go the other way: convert existing bad symbolic
    : permission bit macro use to just use the octal numbers.
    :
    : The symbolic names are good for the *other* bits (ie sticky bit, and the
    : inode mode _type_ numbers etc), but for the permission bits, the symbolic
    : names are just insane crap. Nobody sane should ever use them. Not in the
    : kernel, not in user space.
    (http://lkml.kernel.org/r/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com)

    Link: http://lkml.kernel.org/r/7232ef011d05a92f4caa86a5e9830d87966a2eaf.1470180926.git.joe@perches.com
    Signed-off-by: Joe Perches
    Cc: Linus Torvalds
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Use get_maintainer to check the status of individual files. If
    "obsolete", suggest leaving the files alone.

    Link: http://lkml.kernel.org/r/7ceaa510dc9d2df05ec4b456baed7bb1415550b3.1471889575.git.joe@perches.com
    Signed-off-by: Joe Perches
    Cc: SF Markus Elfring
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

02 Sep, 2016

1 commit


03 Aug, 2016

7 commits

  • If no filenames are given, then read the patch from stdin.

    Link: http://lkml.kernel.org/r/a8784f291ccb5067361992bf5d41ff6cfb0ce5cb.1469830917.git.allenbh@gmail.com
    Signed-off-by: Allen Hubbe
    Acked-by: Joe Perches
    Cc: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Allen Hubbe
     
  • Signoff was not checked if the filename is '-', indicating reading the
    patch from stdin. Commands such as the below would not warn about a
    missing signoff, because the patch filename is '-'. This change allows
    checkpatch to warn about a missing signoff, even if the input filename
    is '-', but only if the patch has a commit message.

    git show --pretty=email | scripts/checkpatch.pl -

    A more common use of checkpatch with stdin is for piping git diff
    through checkpatch. The diff output would not contain a commit message,
    and therefore it would not contain a signoff line. For this common use
    case, a warning should not be printed about the missing signoff. With
    this change we will only warn about a missing signoff if the input
    contains a commit message.

    git diff | scripts/checkpatch.pl -

    Before this patch, a workaround for the first command was to refer to
    stdin by a name other than '-'. The workaround is not an elegant
    solution, because elsewhere checkpatch uses the fact that filename
    equals '-', such as in setting '$vname' to 'Your patch' for stdin. The
    command below would report "/dev/stdin has style problems" instead of
    "Your patch has style problems."

    git show --pretty=email | scripts/checkpatch.pl /dev/stdin

    Link: http://lkml.kernel.org/r/48be31e414bddc65bccfa6b1322359be9ba032eb.1469670589.git.allenbh@gmail.com
    Signed-off-by: Allen Hubbe
    Acked-by: Joe Perches
    Cc: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Allen Hubbe
     
  • Fix false positive warning of identifiers ending in signed with an =
    assignment of WARNING: Prefer 'signed int' to bare use of 'signed'.

    Link: http://lkml.kernel.org/r/6a0e24c3e9102337528ecfcbbe91a0eb5b4820ed.1469529497.git.joe@perches.com
    Signed-off-by: Joe Perches
    Reported-by: Alan Douglas
    Acked-by: Andy Whitcroft
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • BIT macro cannot be exported to UAPI, don't complain about it.

    Link: http://lkml.kernel.org/r/1468707033-16173-1-git-send-email-tomas.winkler@intel.com
    Signed-off-by: Tomas Winkler
    Acked-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Tomas Winkler
     
  • Using \b isn't good enough to isolate what appears to be a commit id in
    a commit message.

    Make sure there is a space or a quote like character after a continuous
    run of hexadecimal characters that could be a commit id.

    Link: http://lkml.kernel.org/r/fdd22b47463a21c21132edbb8aa35e372950a1e6.1468869915.git.joe@perches.com
    Signed-off-by: Joe Perches
    Cc: "Zhuo, Qiuxu"
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Sanitise the lines that contain c99 comments so that the error doesn't
    get emitted.

    Link: http://lkml.kernel.org/r/d4d22c34ad7bcc1bceb52f0742f76b7a6d585235.1468368420.git.joe@perches.com
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • These are also possible single line uses that exceed the generic maximum
    line length (typically 80 columns)

    Link: http://lkml.kernel.org/r/32a6a85fbd6161f1bb55ce176a464e44591afc5b.1468368420.git.joe@perches.com
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

13 Jul, 2016

1 commit

  • The __pmem address space was meant to annotate codepaths that touch
    persistent memory and need to coordinate a call to wmb_pmem(). Now that
    wmb_pmem() is gone, there is little need to keep this annotation.

    Cc: Christoph Hellwig
    Cc: Ross Zwisler
    Signed-off-by: Dan Williams

    Dan Williams
     

04 Jun, 2016

1 commit

  • Some lines in a commit log appear to be commit SHA1 ids like:

    ERROR: Please use git commit description style 'commit ("")' - ie: 'commit 0123456789ab ("commit description")'
    Link: http://lkml.kernel.org/r/40e03fd7aaf1f55c75d787128d6d17c5a71226c2.1464358556.git.vdavydov@virtuozzo.com

    Reduce the false positives.

    Link: http://lkml.kernel.org/r/eda977eaa8328fef42bb3c87935d97e10ea8ff67.1464384023.git.joe@perches.com
    Signed-off-by: Joe Perches
    Reported-by: Andrew Morton
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

21 May, 2016

9 commits

  • The --git shortcut can be confused by a tag with a dash
    like v4.4-rc1.

    Improve the test to verify the expression ends with a
    dash followed by a numeric value.

    Improve the git log result to verify the " " output
    as well.

    Link: http://lkml.kernel.org/r/c4a3f759291d967641860c3a54bb81177f34325f.1462711962.git.joe@perches.com
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • checkpatch currently calls git log multiple times to first get the
    sha1 values and again to get the subject for each
    individual sha1 commit.

    Always get the sha1 and subject at the same time instead. Store the
    subject in a sha1 hash to avoid the second git log exec.

    Link: http://lkml.kernel.org/r/274efab2332ad2308ab5de85a95d255f6e2de5f3.1462711962.git.joe@perches.com
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • It's sometimes useful to scan already committed patches.

    Add --git to scan specific or multiple commits.

    Single commits are scanned with
    --git
    Multiple commits are scanned with
    --git
    --git -

    [joe@perches.com:
    o Don't exec git for each -,
    use a single "git log - "
    o Consolidate the git exec for the and - variants
    o Output 12 character commit hash ids
    o Don't scan git commit merges
    o Use -M to reduce the size of rename commits]

    Signed-off-by: "Du, Changbin"
    Signed-off-by: Joe Perches
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Du, Changbin
     
  • The message types are not currently knowable without reading the code.
    Add a mechanism to see what they are.

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

    Joe Perches
     
  • The --fix option is relatively unknown and underutilized.

    Add some text to show that it's available when style defects are found.

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

    Joe Perches
     
  • Add a test for use of ACCESS_ONCE that could be written using READ_ONCE or
    WRITE_ONCE.

    --fix it too if desired.

    The WRITE_ONCE fixes are less correct than the coccinelle script below as
    checkpatch cannot have a completely correct "expression" mechanism because
    checkpatch works on patches and not complete files.

    $ cat access_once.cocci
    @@
    expression e1;
    expression e2;
    @@

    - ACCESS_ONCE(e1) = e2
    + WRITE_ONCE(e1, e2)

    @@
    expression e1;
    @@

    - ACCESS_ONCE(e1)
    + READ_ONCE(e1)

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

    Joe Perches
     
  • It's somewhat common and in general a defect for c90 keywords to
    not start on a tabstop.

    Add a test for this condition and warn when it occurs.

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

    Joe Perches
     
  • A "." dereference to an all uppercase structure member can be
    incorrectly reported as a CONSTANT_COMPARISON.

    ie: "if (table[i].PANELID == tempdx)"

    Fix it by checking for "." before the constant test.

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

    Joe Perches
     
  • Using #if defined CONFIG_ || defined CONFIG__MODULE is
    more verbose than necessary and IS_ENABLED(CONFIG_) is preferred.

    So add a test and a message for it.

    --fix it to if desired.

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

    Joe Perches
     

17 Mar, 2016

1 commit

  • Merge first patch-bomb from Andrew Morton:

    - some misc things

    - ofs2 updates

    - about half of MM

    - checkpatch updates

    - autofs4 update

    * emailed patches from Andrew Morton : (120 commits)
    autofs4: fix string.h include in auto_dev-ioctl.h
    autofs4: use pr_xxx() macros directly for logging
    autofs4: change log print macros to not insert newline
    autofs4: make autofs log prints consistent
    autofs4: fix some white space errors
    autofs4: fix invalid ioctl return in autofs4_root_ioctl_unlocked()
    autofs4: fix coding style line length in autofs4_wait()
    autofs4: fix coding style problem in autofs4_get_set_timeout()
    autofs4: coding style fixes
    autofs: show pipe inode in mount options
    kallsyms: add support for relative offsets in kallsyms address table
    kallsyms: don't overload absolute symbol type for percpu symbols
    x86: kallsyms: disable absolute percpu symbols on !SMP
    checkpatch: fix another left brace warning
    checkpatch: improve UNSPECIFIED_INT test for bare signed/unsigned uses
    checkpatch: warn on bare unsigned or signed declarations without int
    checkpatch: exclude asm volatile from complex macro check
    mm: memcontrol: drop unnecessary lru locking from mem_cgroup_migrate()
    mm: migrate: consolidate mem_cgroup_migrate() calls
    mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous
    ...

    Linus Torvalds
     

16 Mar, 2016

4 commits

  • This patch escapes a regex that uses left brace.

    Using checkpatch.pl with Perl 5.22.0 generates the warning: "Unescaped
    left brace in regex is deprecated, passed through in regex;"

    Comment from regcomp.c in Perl source: "Currently we don't warn when the
    lbrace is at the start of a construct. This catches it in the middle of
    a literal string, or when it's the first thing after something like
    "\b"."

    This works as a complement to 4e5d56bd ("checkpatch: fix left brace
    warning").

    Signed-off-by: Geyslan G. Bem
    Signed-off-by: Joe Perches
    Suggested-by: Peter Senna Tschudin
    Cc: Eddie Kovsky
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Geyslan G. Bem
     
  • Improve the test to allow casts to (unsigned) or (signed) to be found
    and fixed if desired.

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

    Joe Perches
     
  • Kernel style prefers "unsigned int " over "unsigned " and
    "signed int " over "signed ".

    Emit a warning for these simple signed/unsigned declarations. Fix
    it too if desired.

    Signed-off-by: Joe Perches
    Acked-by: David S. Miller
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • asm volatile and all its variants like __asm__ __volatile__ ("")
    are reported as errors with "Macros with with complex values should be
    enclosed in parentheses".

    Make an exception for these asm volatile macro definitions by converting
    the "asm volatile" to "asm_volatile" so it appears as a single function
    call and the error isn't reported.

    Signed-off-by: Joe Perches
    Reported-by: Jeff Merkey
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     

24 Feb, 2016

1 commit

  • In C programming language, we don't have a easy way to privatize a
    member of a structure. However in kernel, sometimes there is a need to
    privatize a member in case of potential bugs or misuses.

    Fortunately, the noderef attribute of sparse is a way to privatize a
    member, as by defining a member as noderef, the address-of operator on
    the member will produce a noderef pointer to that member, and if anyone
    wants to dereference that kind of pointers to read or modify the member,
    sparse will yell.

    Based on this, __private modifier and related operation ACCESS_PRIVATE()
    are introduced, which could help detect undesigned public uses of
    private members of structs. Here is an example of sparse's output if it
    detect an undersigned public use:

    | kernel/rcu/tree.c:4453:25: warning: incorrect type in argument 1 (different modifiers)
    | kernel/rcu/tree.c:4453:25: expected struct raw_spinlock [usertype] *lock
    | kernel/rcu/tree.c:4453:25: got struct raw_spinlock [noderef] *

    Also, this patch improves compiler.h a little bit by adding comments for
    "#else" and "#endif".

    Signed-off-by: Boqun Feng
    Signed-off-by: Paul E. McKenney

    Boqun Feng
     

21 Jan, 2016

3 commits

  • A simple search over the kernel souce displays a number of correctly
    defined multiline macro, which generally are used as an array element
    initializer:

    % find ../linux -type f | xargs grep -B1 -H '^[:space]*\[.*\\$'

    However checkpatch.pl unexpectedly complains about all these macro
    definitions:

    % ./scripts/checkpatch.pl --types COMPLEX_MACRO -f include/linux/perf/arm_pmu.h

    ERROR: Macros with complex values should be enclosed in parentheses
    +#define PERF_MAP_ALL_UNSUPPORTED \
    + [0 ... PERF_COUNT_HW_MAX - 1] = HW_OP_UNSUPPORTED

    The change intends to fix this type of false positives by flattening
    only array members and skipping array element designators.

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

    Vladimir Zapolskiy
     
  • The current test excludes any macro with ## concatenation from being
    reported with hidden flow control.

    Some macros are used with return or goto statements along with ##args or
    ##__VA_ARGS__. A somewhat common case is a logging macro like
    pr_info(fmt, ...) then a return or goto statement.

    Check the concatenated variable for args or __VA_ARGS__ and allow those
    macros to also be reported when they contain a return or goto.

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

    Joe Perches
     
  • Linus Torvalds wrote:

    > I can't but help to react that this:
    > #define IOMMU_ERROR_CODE (~(unsigned long) 0)
    > Not that this *matters*, but it's a bit odd to have to cast constants
    > to perfectly regular C types.

    So add a test that looks for constants that are cast to
    standard C90 int or longer types and suggest using C90
    "6.4.4.1 Integer constants" integer-suffixes instead.

    Miscellanea:

    o Add a --fix option too

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

    Joe Perches