checkpatch: don't require octal permissions for "0"

PATCH In review Bartosz Golaszewski 2 participants 4 messages Dec 10, 2017 – Dec 11, 2017 Checkpatch

Share & tools

Bartosz Golaszewski

author

2017-12-10 14:04 UTC #0 lore raw

PATCH 1/1 checkpatch: don't require octal permissions for "0"

If the permission value is 0, don't raise the NON_OCTAL_PERMISSIONS error. There's no possibility of an error if there are no permissions.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/checkpatch.pl +1 -1

index 6b130a4116fa..ea98e298d3ac 100755
@@ -6321,7 +6321,7 @@ sub process {
 				if ($stat =~ /$test/) {
 					my $val = $1;
 					$val = $6 if ($skip_args ne "");
-					if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
+					if (($val ne "0" && $val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
 					    ($val =~ /^$Octal$/ && length($val) ne 4)) {
 						ERROR("NON_OCTAL_PERMISSIONS",
 						      "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real);

Signature · 1 line

2.15.1

Joe Perches

maintainer

2017-12-10 22:56 UTC #1 lore raw

Quoted 2 lines · Bartosz Golaszewski

If the permission value is 0, don't raise the NON_OCTAL_PERMISSIONS error. There's no possibility of an error if there are no permissions.

Hi Bartosz

This response below is the same as another email thread

-------------

Personally, I prefer 4 digit octal in most cases as it shows the coder knows that the argument is a permissions use and not just some generic 0.

There are not many uses of 0 for permissions outside of module_param*.

I suppose all the variants of module_param calls, as a 0 there is specifically a "not to appear in sysfs" flag, could or should be excluded from that octal test.

Joe Perches

maintainer

2017-12-11 02:19 UTC #2 lore raw

In reply to Bartosz Golaszewski

Quoted 14 lines · Bartosz Golaszewski

If the permission value is 0, don't raise the NON_OCTAL_PERMISSIONS error. There's no possibility of an error if there are no permissions.
Suggested-by Andy Shevchenko <[email protected]>
Signed-off-by Bartosz Golaszewski <[email protected]>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/checkpatch.pl +0 -0
index 6b130a4116fa..ea98e298d3ac 100755

scripts/checkpatch.pl sub process { · line 6327

Quoted 8 lines

scripts/checkpatch.pl +1 -1

@@ -6321,7 +6321,7 @@ sub process {
 				if ($stat =~ /$test/) {
 					my $val = $1;
 					$val = $6 if ($skip_args ne "");
-					if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
+					if (($val ne "0" && $val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
 					    ($val =~ /^$Octal$/ && length($val) ne 4)) {
 						ERROR("NON_OCTAL_PERMISSIONS",
 						      "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real);

perhaps
---

scripts/checkpatch.pl +2 -1

index 6b130a4116fa..950bbc0e6e3f 100755
@@ -6321,7 +6321,8 @@ sub process {
 				if ($stat =~ /$test/) {
 					my $val = $1;
 					$val = $6 if ($skip_args ne "");
-					if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
+					if ($func !~ /^module_param/ &&
+					    ($val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
 					    ($val =~ /^$Octal$/ && length($val) ne 4)) {
 						ERROR("NON_OCTAL_PERMISSIONS",
 						      "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real);

Bartosz Golaszewski

author

2017-12-11 09:39 UTC #3 lore raw

2017-12-11 3:19 GMT+01:00 Joe Perches <[email protected]>:

Quoted 31 lines · Bartosz Golaszewski

Quoted 14 lines

If the permission value is 0, don't raise the NON_OCTAL_PERMISSIONS error. There's no possibility of an error if there are no permissions.
Suggested-by Andy Shevchenko <[email protected]>
Signed-off-by Bartosz Golaszewski <[email protected]>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/checkpatch.pl +0 -0
index 6b130a4116fa..ea98e298d3ac 100755

scripts/checkpatch.pl sub process { · line 6327

Quoted 8 lines

scripts/checkpatch.pl +1 -1

@@ -6321,7 +6321,7 @@ sub process {
                              if ($stat =~ /$test/) {
                                      my $val = $1;
                                      $val = $6 if ($skip_args ne "");
-                                     if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
+                                     if (($val ne "0" && $val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
                                          ($val =~ /^$Octal$/ && length($val) ne 4)) {
                                              ERROR("NON_OCTAL_PERMISSIONS",
                                                    "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real);

perhaps
---

scripts/checkpatch.pl +0 -0

index 6b130a4116fa..950bbc0e6e3f 100755

scripts/checkpatch.pl sub process { · line 6328

Quoted 9 lines

scripts/checkpatch.pl +2 -1

@@ -6321,7 +6321,8 @@ sub process {
                                if ($stat =~ /$test/) {
                                        my $val = $1;
                                        $val = $6 if ($skip_args ne "");
-                                       if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
+                                       if ($func !~ /^module_param/ &&
+                                           ($val =~ /^$Int$/ && $val !~ /^$Octal$/) ||
                                            ($val =~ /^$Octal$/ && length($val) ne 4)) {
                                                ERROR("NON_OCTAL_PERMISSIONS",
                                                      "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real);

I would still check for octal permissions for anything else but "0" here. I'll send a patch shortly.

Thanks,
Bartosz