checkpatch: check for incorrect permissions
in [Kernel]
|
Prev: [tip:x86/cleanups] x86, cleanup: Remove obsolete boot_cpu_id variable
Next: [PATCH 1/2] tracing: Extend recordmcount to better support Blackfin mcount
From: Andrew Morton on 12 Aug 2010 17:10 On Thu, 12 Aug 2010 10:45:45 +0530 Rabin Vincent <rabin.vincent(a)stericsson.com> wrote: > Throw an error when a source file has been given execute permissions > using the mode change line present in git diffs. Also alow the filename > matching to use the "diff" line in addition to the "+++" line, since the > mode change lines appear before any "+++" lines. > > Cc: Andy Whitcroft <apw(a)canonical.com> > Acked-by: Linus Walleij <linus.walleij(a)stericsson.com> > Signed-off-by: Rabin Vincent <rabin.vincent(a)stericsson.com> > --- > scripts/checkpatch.pl | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 2039acd..4c35cb9 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1308,7 +1308,7 @@ sub process { > $here = "#$realline: " if ($file); > > # extract the filename as it passes > - if ($line=~/^\+\+\+\s+(\S+)/) { > + if ($line=~/^\+\+\+\s+(\S+)/ || $line=~/^diff\s.*?(\S+)$/) { Breakage. This causes the false warning: WARNING: patch prefix 'drivers' exists, appears to be a -p0 patch to be emitted when processing the below patch: From: Dan Carpenter <error27(a)gmail.com> copy_to_user() returns the number of bytes remaining but we want to return a negative error code on errors. Signed-off-by: Dan Carpenter <error27(a)gmail.com> Cc: "Michael H. Warfield" <mhw(a)wittsend.com> Cc: Greg KH <greg(a)kroah.com> Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org> --- drivers/char/ip2/ip2main.c | 2 ++ 1 file changed, 2 insertions(+) diff -puN drivers/char/ip2/ip2main.c~ip2-return-efault-on-copy_to_user-errors drivers/char/ip2/ip2main.c --- a/drivers/char/ip2/ip2main.c~ip2-return-efault-on-copy_to_user-errors +++ a/drivers/char/ip2/ip2main.c @@ -2930,6 +2930,8 @@ ip2_ipl_ioctl (struct file *pFile, UINT if ( pCh ) { rc = copy_to_user(argp, pCh, sizeof(i2ChanStr)); + if (rc) + rc = -EFAULT; } else { rc = -ENODEV; } _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rabin VINCENT on 13 Aug 2010 01:00
On Thu, Aug 12, 2010 at 23:02:22 +0200, Andrew Morton wrote: > On Thu, 12 Aug 2010 10:45:45 +0530 > Rabin Vincent <rabin.vincent(a)stericsson.com> wrote: > > # extract the filename as it passes > > - if ($line=~/^\+\+\+\s+(\S+)/) { > > + if ($line=~/^\+\+\+\s+(\S+)/ || $line=~/^diff\s.*?(\S+)$/) { > > Breakage. This causes the false warning: > > WARNING: patch prefix 'drivers' exists, appears to be a -p0 patch I've fixed this by matching for the filename in the diff line only when it's "diff --git". Also prevented a double-print of the "do not modify file in include/asm" error. From b854ea24f1bd5391a63d8d4e2aa63e1c10870816 Mon Sep 17 00:00:00 2001 From: Rabin Vincent <rabin.vincent(a)stericsson.com> Date: Wed, 11 Aug 2010 14:45:45 +0530 Subject: [PATCHv2] checkpatch: check for incorrect permissions Throw an error when a source file has been given execute permissions using the mode change line present in git diffs. Also alow the filename matching to use the "diff" line in addition to the "+++" line, since the mode change lines appear before any "+++" lines. Cc: Andy Whitcroft <apw(a)canonical.com> Acked-by: Linus Walleij <linus.walleij(a)stericsson.com> Signed-off-by: Rabin Vincent <rabin.vincent(a)stericsson.com> --- scripts/checkpatch.pl | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2039acd..a6858be 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1308,7 +1308,7 @@ sub process { $here = "#$realline: " if ($file); # extract the filename as it passes - if ($line=~/^\+\+\+\s+(\S+)/) { + if ($line =~ /^\+\+\+\s+(\S+)/ || $line =~ /^diff --git.*?(\S+)$/) { $realfile = $1; $realfile =~ s@^([^/]*)/@@; @@ -1318,6 +1318,10 @@ sub process { WARN("patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n"); } + # filename based errors will anyway get printed when we + # get to the context lines + next if ($line =~ /^diff/); + if ($realfile =~ m@^include/asm/@) { ERROR("do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n"); } @@ -1332,6 +1336,14 @@ sub process { $cnt_lines++ if ($realcnt != 0); +# Check for incorrect file permissions + if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { + my $permhere = $here . "FILE: $realfile\n"; + if ($realfile =~ /(Makefile|Kconfig|\.c|\.h|\.S|\.tmpl)$/) { + ERROR("do not set execute permissions for source files\n" . $permhere); + } + } + #check the patch for a signoff: if ($line =~ /^\s*signed-off-by:/i) { # This is a signoff, if ugly, so do not double report. -- 1.7.2.dirty -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |