dts and dtsi files should not be given +x permissions.
Change-Id: I23fa5d4012f935f64975471bd6c023b141cde45b
Signed-off-by: Michael Bohan <mbohan@codeaurora.org>
Rather than forcing authors of reverts to truncate the summary line
because the "Revert" prefix added pushes it over the character limit,
excuse reverts from this rule.
Change-Id: I395dfff3327e360ef935d4a685c38df6577e3867
Signed-off-by: Matt Wagantall <mattw@codeaurora.org>
Software security is an important issue, in general, but it is
especially important in Linux kernel code. Buffer overflows can have
wide-reaching ramifications and can often be readily exploited to
compromise the entire system. It is important for every developer to
be aware of security issues while writing code.
However, I've noticed a few "rules" about coding that are resulting in
code that isn't any more secure, and has the disadvantage of obscuring
what the code is doing. In most instances, the "corrected" code is
actually wrong: we've traded a perceived lack of safety for incorrect
behavior. These obfuscations also make this code more distant from
upstream kernel standards.
I'm only going to focus here on strcmp/strncmp and strlen/strnlen. I
choose these two, because in the context of the kernel, it's not easy
to make a general rule, such as "always use the 'n' variant". These
function have different behavior, and the 'n' isn't just a blanket fix
that makes them better. In many instances, the correct call is the
plain variant (strcpy has a strlcpy variant which is usually helpful).
Let's start with strlen/strnlen.
size_t strlen(const char *);
size_t strnlen(const char *, size_t);
The strlen() function scans for a NUL byte within a string, and
returns the number of characters that had to be skipped to get there.
The strnlen() call is similar, but will stop after after a maximal
number of characters, and return that result. This variant was
intended for storing variable length strings in fixed-sized buffers,
where the full-length case did not have a trailing NUL. This storage
model is very uncommon, as is the use of strnlen().
The question becomes, what is the maximal length you should be giving
to strnlen(). If the string is truly variable length (allocated and
filled), there really isn't a meaningful value to use for this. The
only time that a max length makes sense is when you have something
like:
char name[MAX_NAME_LENGTH];
but, in this case, strnlen() is still probably not what you want to be
using. It would be safe to use, if you check the result, and if it is
MAX_NAME_LENGTH, raise some kind of error case. If later code assumes
there is a NUL at the end, there will still be a buffer overflow. In
this case, it is much better to check the length before storing it in
this field, and make sure there is room for the NUL.
If the string is a constant, passing in a length doesn't make sense,
since you would have to know the length of the string to check that.
There is no safety issue with calling strlen() on a constant.
So, the simple rule for strnlen()/strlen() is:
- If the string doesn't have an obvious bound length, such as an
allocated string, use strlen().
- If the string is a constant, use strlen().
- If there is a fixed buffer, strnlen() might make sense, but it is
probably better to change the design to avoid these types of
strings.
The only case where strnlen really makes sense is when you have a
string that is passed in from the user. In this case, it is very
important to check the result, and if the length is at the maximum,
return an error, and don't try to do any processing on the string.
Moving on to strcmp/strncmp. These functions are similar, except that
they take two string arguments, which gives a lot more combinations.
int strcmp(const char *, const char *);
int strncmp(const char *, const char *, size_t);
These will walk both strings (at the same time), and stop when
reaching the first difference between them. This means that they will
never go further than the length of the shortest string being
compared. As in strnlen, the max argument to strncmp sets a limit on
the comparison. Similar to strnlen, the results are unusual when the
limit is reached, but in a sense, even worse, since it may consider
the strings equal without ever reaching the end of either.
Looking over the 200 some uses of strncmp in the msm code, almost all
of them do something akin to:
strncmp(value, constant, strlen(constant))
If the call has added 1 to the 'strlen' result, the strncmp would just
become an inefficient, but otherwise identical version of strcmp.
However, without the +1, this compares the prefix of 'value' instead
of the entire string. Only one instance of strncmp in the code
appears to be intentionally checking for a prefix. The rest have
changed a simple string compare into an unintentional prefix match.
Because there are two strings, it is a little more complex to
determine which to use, but it is very similar. It might seem that
strncmp() would be useful for checking an unknown buffer (say from
userspace). However, since strncmp()'s result doesn't distinguish
between finding the end of the string, or hitting the max, there's no
way to know. Some guidelines:
- If one of the strings has a known bound length (such as a
constant, or another string whose length has already been
checked), AND this bound length is within the expected buffer of
the other string, it is safe to use strcmp().
- Otherwise, you may need to use something like strnlen() to
determine a maximum length before calling strcmp().
- strncmp() is useful to test a string for a prefix. No other uses
make sense.
To facilitate fixing these, remove strlen(), strcmp(), and
strcasecmp() from the list of calls that are banned. Problems with
these calls need to be caught at a higher level (such as review), and
replacing them with the 'n' variants doesn't help anything.
This will be followed by some patches that fixup the incorrect code
introduced by this "ban".
Change-Id: I77dfe1f2f58e8c951e4b38b23f4ec79f8209b1dc
Signed-off-by: David Brown <davidb@codeaurora.org>
Many messsages are missing the new message type parameter, which
causes warning messages and a lack of the line number from the
offending line in the patch.
Change-Id: I69f2283c3dc27edd66fd2676c8be45664699dba6
Signed-off-by: Steve Muckle <smuckle@codeaurora.org>
AU_LINUX_ANDROID_ICS.04.00.04.00.126 from msm-3.0.
First parent is from google/android-3.4.
* commit 'AU_LINUX_ANDROID_ICS.04.00.04.00.126': (8712 commits)
PRNG: Device tree entry for qrng device.
vidc:1080p: Set video core timeout value for Thumbnail mode
msm: sps: improve the debugging support in SPS driver
board-8064 msm: Overlap secure and non secure video firmware heaps.
msm: clock: Add handoff ops for 7x30 and copper XO clocks
msm_fb: display: Wait for external vsync before DTV IOMMU unmap
msm: Fix ciruclar dependency in debug UART settings
msm: gdsc: Add GDSC regulator driver for msm-copper
defconfig: Enable Mobicore Driver.
mobicore: Add mobicore driver.
mobicore: rename variable to lower case.
mobicore: rename folder.
mobicore: add makefiles
mobicore: initial import of kernel driver
ASoC: msm: Add SLIMBUS_2_RX CPU DAI
board-8064-gpio: Update FUNC for EPM SPI CS
msm_fb: display: Remove chicken bit config during video playback
mmc: msm_sdcc: enable the sanitize capability
msm-fb: display: lm2 writeback support on mpq platfroms
msm_fb: display: Disable LVDS phy & pll during panel off
...
Signed-off-by: Steve Muckle <smuckle@codeaurora.org>
Revert the --strict test for the old preferred block
comment style in drivers/net and net/
Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <josh@joshtriplett.org>
Acked-by: Joe Perches <joe@perches.com>
Cc: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <joe@perches.com>
Cc: Andy Whitcroft <apw@shadowen.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <artem.bityutskiy@linux.intel.com>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <joe@perches.com>
Cc: "Bruce W. Allen" <bruce.w.allan@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
There are many string based functions that are unsafe
to use. Some of the functions are unsafe to use because
of the possibility of overflow while others cannot
guarantee that the resultant string is NULL-terminated.
Add check for these functions and log message indicating
which safe functions can be used instead.
Change-Id: Id305d98df241e3fd257529529739dcd4f3659186
Signed-off-by: Olav Haugan <ohaugan@codeaurora.org>
There are some very frequently used tags that checkpatch can look
for as an ending to the commit text rather than using a pattern,
which can generate false positives in the "no commit text" rule.
Change-Id: I5b4400017b8273bcd9f5a59b3e28965c0062bef4
Signed-off-by: Steve Muckle <smuckle@codeaurora.org>
Commit text is almost always necessary to explain why a change is
needed. Exceptions are rare enough that these can be granted through
manual checkpatch overrides.
Change-Id: I926b2276f717940c2fec77a6709fa3088b1bf0c3
Signed-off-by: Steve Muckle <smuckle@codeaurora.org>
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 <stdio.h>
#include <stdlib.h>
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 <joe@perches.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <rostedt@goodmis.org>
Tested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <joe@perches.com>
Acked-by: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
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 <joe@perches.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Vreg API implementation is deprecated. We are using
Linux regulator framework now. Hence vreg API should
not be used.
Change-Id: I8e31dac66592d2d195d190b770a436e93206cf8b
Signed-off-by: Pankaj Kumar <pakuma@codeaurora.org>
MSM has a board-*-gpiomux file where all the gpiomux configs reside.
Warn if a non gpiomux board file tries to add gpiomux configs.
The camera board file is an exception to this rule.
Change-Id: Ibab190dcbd7ea78e7ca150142c68c5ae881e4e06
Signed-off-by: Rohit Vaswani <rvaswani@codeaurora.org>
* 'misc' of git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild:
script/checkpatch.pl: warn about deprecated use of EXTRA_{A,C,CPP,LD}FLAGS
tags, powerpc: Update tags.sh to support _GLOBAL symbols
scripts: add extract-vmlinux
Some find using utf-8 in commit logs inappropriate.
Some patch commit logs contain unintended utf-8 characters when doing
things like copy/pasting compilation output.
Look for the start of any commit log by skipping initial lines that look
like email headers and "From: " lines.
Stop looking for utf-8 at the first signature line.
Signed-off-by: Joe Perches <joe@perches.com>
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Whitcroft <apw@shadowen.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Mark obsolete/deprecated strict_strto<foo> and simple_strto<foo> functions
and macros as obsolete.
Update checkpatch to warn about their use.
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Existing executable-bit test only works on second and later files
contained in the patch. Correct this so all patches in the file
are tested.
Change-Id: Ie9363473f0d2fc067f9c593c86495d15e8e5d546
Signed-off-by: Gregory Bean <gbean@codeaurora.org>
While this check is valid in theory, it generates a plethora
of false positives on ALSA drivers and ends up causing more trouble
than it saves.
Signed-off-by: Gregory Bean <gbean@codeaurora.org>
The patch-header cleanup code assumed that it would only ever
see patches, which was of course hogwash. This lead to crazy
results as it tried to wrap what it thought were continuation
lines at the beginnings of raw diffs.
Signed-off-by: Gregory Bean <gbean@codeaurora.org>
This is a resend from the original, changing the title from PATCH to
RFC(since this is a review for commit, and I should have put that the first go around).
and also removing some of the commit's with ia64 and bash since it is significant.
let me know if I might have missed anything etc..
Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Usage of these flags has been deprecated for nearly 4 years by:
commit f77bf01425
Author: Sam Ravnborg <sam@neptun.(none)>
Date: Mon Oct 15 22:25:06 2007 +0200
kbuild: introduce ccflags-y, asflags-y and ldflags-y
Moreover, these flags (at least EXTRA_CFLAGS) have been documented for command
line use. By default, gmake(1) do not override command line setting, so this is
likely to result in build failure or unexpected behavior.
Warn about their introduction in Makefile or Kbuild files.
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
Signed-off-by: Michal Marek <mmarek@suse.cz>
Michael Bohan
Matt Wagantall
David Brown
Steve Muckle
Joe Perches
Josh Triplett
Andy Whitcroft
Artem Bityutskiy
Olav Haugan
Pankaj Kumar
Rohit Vaswani
Linus Torvalds
Gregory Bean
Bryan Huntsman
Jiri Kosina
Justin P. Mattock
Arnaud Lacombe
Hui Zhu