checkpatch: add a few DEVICE_ATTR style tests

DEVICE_ATTR is a declaration macro that has a few alternate and
preferred forms like DEVICE_ATTR_RW, DEVICE_ATTR_RO, and DEVICE_ATTR.

As well, many uses of DEVICE_ATTR could use the preferred forms when the
show or store functions are also named in a regular form.

Suggest the preferred forms when appropriate.

Also emit a permissions warning if the the permissions are not the
typical 0644, 0444, or 0200.

Link: http://lkml.kernel.org/r/725864f363d91d1e1e6894a39fb57662eabd6d65.1513803306.git.joe@perches.com
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>

1 files changed, 93 insertions, 21 deletions

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7c635146cb80..51905634be23 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -566,6 +566,7 @@ foreach my $entry (@mode_permission_funcs) {
         $mode_perms_search .= '|' if ($mode_perms_search ne "");
         $mode_perms_search .= $entry->[0];
 }
+$mode_perms_search = "(?:${mode_perms_search})";
 
 our $mode_perms_world_writable = qr{
         S_IWUGO         |
@@ -600,6 +601,37 @@ foreach my $entry (keys %mode_permission_string_types) {
         $mode_perms_string_search .= '|' if ($mode_perms_string_search ne "");
         $mode_perms_string_search .= $entry;
 }
+our $single_mode_perms_string_search = "(?:${mode_perms_string_search})";
+our $multi_mode_perms_string_search = qr{
+        ${single_mode_perms_string_search}
+        (?:\s*\|\s*${single_mode_perms_string_search})*
+}x;
+
+sub perms_to_octal {
+        my ($string) = @_;
+
+        return trim($string) if ($string =~ /^\s*0[0-7]{3,3}\s*$/);
+
+        my $val = "";
+        my $oval = "";
+        my $to = 0;
+        my $curpos = 0;
+        my $lastpos = 0;
+        while ($string =~ /\b(($single_mode_perms_string_search)\b(?:\s*\|\s*)?\s*)/g) {
+                $curpos = pos($string);
+                my $match = $2;
+                my $omatch = $1;
+                last if ($lastpos > 0 && ($curpos - length($omatch) != $lastpos));
+                $lastpos = $curpos;
+                $to |= $mode_permission_string_types{$match};
+                $val .= '\s*\|\s*' if ($val ne "");
+                $val .= $match;
+                $oval .= $omatch;
+        }
+        $oval =~ s/^\s*\|\s*//;
+        $oval =~ s/\s*\|\s*$//;
+        return sprintf("%04o", $to);
+}
 
 our $allowed_asm_includes = qr{(?x:
         irq|
@@ -6274,6 +6306,63 @@ sub process {
                              "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr);
                 }
 
+# check for DEVICE_ATTR uses that could be DEVICE_ATTR_<FOO>
+# and whether or not function naming is typical and if
+# DEVICE_ATTR permissions uses are unusual too
+                if ($^V && $^V ge 5.10.0 &&
+                    defined $stat &&
+                    $stat =~ /\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?\s*(\s*(?:${multi_mode_perms_string_search}|0[0-7]{3,3})\s*)\s*\)?\s*,\s*(\w+)\s*,\s*(\w+)\s*\)/) {
+                        my $var = $1;
+                        my $perms = $2;
+                        my $show = $3;
+                        my $store = $4;
+                        my $octal_perms = perms_to_octal($perms);
+                        if ($show =~ /^${var}_show$/ &&
+                            $store =~ /^${var}_store$/ &&
+                            $octal_perms eq "0644") {
+                                if (WARN("DEVICE_ATTR_RW",
+                                         "Use DEVICE_ATTR_RW\n" . $herecurr) &&
+                                    $fix) {
+                                        $fixed[$fixlinenr] =~ s/\bDEVICE_ATTR\s*\(\s*$var\s*,\s*\Q$perms\E\s*,\s*$show\s*,\s*$store\s*\)/DEVICE_ATTR_RW(${var})/;
+                                }
+                        } elsif ($show =~ /^${var}_show$/ &&
+                                 $store =~ /^NULL$/ &&
+                                 $octal_perms eq "0444") {
+                                if (WARN("DEVICE_ATTR_RO",
+                                         "Use DEVICE_ATTR_RO\n" . $herecurr) &&
+                                    $fix) {
+                                        $fixed[$fixlinenr] =~ s/\bDEVICE_ATTR\s*\(\s*$var\s*,\s*\Q$perms\E\s*,\s*$show\s*,\s*NULL\s*\)/DEVICE_ATTR_RO(${var})/;
+                                }
+                        } elsif ($show =~ /^NULL$/ &&
+                                 $store =~ /^${var}_store$/ &&
+                                 $octal_perms eq "0200") {
+                                if (WARN("DEVICE_ATTR_WO",
+                                         "Use DEVICE_ATTR_WO\n" . $herecurr) &&
+                                    $fix) {
+                                        $fixed[$fixlinenr] =~ s/\bDEVICE_ATTR\s*\(\s*$var\s*,\s*\Q$perms\E\s*,\s*NULL\s*,\s*$store\s*\)/DEVICE_ATTR_WO(${var})/;
+                                }
+                        } elsif ($octal_perms eq "0644" ||
+                                 $octal_perms eq "0444" ||
+                                 $octal_perms eq "0200") {
+                                my $newshow = "$show";
+                                $newshow = "${var}_show" if ($show ne "NULL" && $show ne "${var}_show");
+                                my $newstore = $store;
+                                $newstore = "${var}_store" if ($store ne "NULL" && $store ne "${var}_store");
+                                my $rename = "";
+                                if ($show ne $newshow) {
+                                        $rename .= " '$show' to '$newshow'";
+                                }
+                                if ($store ne $newstore) {
+                                        $rename .= " '$store' to '$newstore'";
+                                }
+                                WARN("DEVICE_ATTR_FUNCTIONS",
+                                     "Consider renaming function(s)$rename\n" . $herecurr);
+                        } else {
+                                WARN("DEVICE_ATTR_PERMS",
+                                     "DEVICE_ATTR unusual permissions '$perms' used\n" . $herecurr);
+                        }
+                }
+
 # Mode permission misuses where it seems decimal should be octal
 # This uses a shortcut match to avoid unnecessary uses of a slow foreach loop
 # o Ignore module_param*(...) uses with a decimal 0 permission as that has a
@@ -6318,30 +6407,13 @@ sub process {
                 }
 
 # check for uses of S_<PERMS> that could be octal for readability
-                if ($line =~ /\b$mode_perms_string_search\b/) {
-                        my $val = "";
-                        my $oval = "";
-                        my $to = 0;
-                        my $curpos = 0;
-                        my $lastpos = 0;
-                        while ($line =~ /\b(($mode_perms_string_search)\b(?:\s*\|\s*)?\s*)/g) {
-                                $curpos = pos($line);
-                                my $match = $2;
-                                my $omatch = $1;
-                                last if ($lastpos > 0 && ($curpos - length($omatch) != $lastpos));
-                                $lastpos = $curpos;
-                                $to |= $mode_permission_string_types{$match};
-                                $val .= '\s*\|\s*' if ($val ne "");
-                                $val .= $match;
-                                $oval .= $omatch;
-                        }
-                        $oval =~ s/^\s*\|\s*//;
-                        $oval =~ s/\s*\|\s*$//;
-                        my $octal = sprintf("%04o", $to);
+                if ($line =~ /\b($multi_mode_perms_string_search)\b/) {
+                        my $oval = $1;
+                        my $octal = perms_to_octal($oval);
                         if (WARN("SYMBOLIC_PERMS",
                                  "Symbolic permissions '$oval' are not preferred. Consider using octal permissions '$octal'.\n" . $herecurr) &&
                             $fix) {
-                                $fixed[$fixlinenr] =~ s/$val/$octal/;
+                                $fixed[$fixlinenr] =~ s/\Q$oval\E/$octal/;
                         }
                 }