* [RFC] Resolving "false positive" error message from checkpatch.pl
@ 2017-02-03 10:11 Slawomir Stepien
2017-02-03 10:27 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Slawomir Stepien @ 2017-02-03 10:11 UTC (permalink / raw)
To: apw, joe; +Cc: linux-iio, gregkh, devel
Hi all
There is a "false positive" error reported by checkpatch.pl:
ERROR: Use 4 digit octal (0777) not decimal permissions
#272: FILE: drivers/staging/iio/meter/ade7759.c:272:
+static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO,
+ ade7759_read_8bit,
+ ade7759_write_8bit,
+ ADE7759_CH1OS);
ERROR: Use 4 digit octal (0777) not decimal permissions
#276: FILE: drivers/staging/iio/meter/ade7759.c:276:
+static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO,
+ ade7759_read_8bit,
+ ade7759_write_8bit,
+ ADE7759_CH2OS);
The same for the file drivers/staging/iio/meter/ade7753.c.
We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from
@mode_permission_funcs in checkpatch.pl.
My question is: how this should be fixed?
>From what I can say, it's a false positive so the correct way to fix it is to
change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+".
Or maybe the fix should be done in iio code - argument position inside this
macro?
--
Slawomir Stepien
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC] Resolving "false positive" error message from checkpatch.pl 2017-02-03 10:11 [RFC] Resolving "false positive" error message from checkpatch.pl Slawomir Stepien @ 2017-02-03 10:27 ` Greg KH 2017-02-03 12:10 ` Slawomir Stepien 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2017-02-03 10:27 UTC (permalink / raw) To: Slawomir Stepien; +Cc: apw, joe, linux-iio, devel On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote: > Hi all > > There is a "false positive" error reported by checkpatch.pl: > > ERROR: Use 4 digit octal (0777) not decimal permissions > #272: FILE: drivers/staging/iio/meter/ade7759.c:272: > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO, > + ade7759_read_8bit, > + ade7759_write_8bit, > + ADE7759_CH1OS); > > ERROR: Use 4 digit octal (0777) not decimal permissions > #276: FILE: drivers/staging/iio/meter/ade7759.c:276: > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO, > + ade7759_read_8bit, > + ade7759_write_8bit, > + ADE7759_CH2OS); > > The same for the file drivers/staging/iio/meter/ade7753.c. > > We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from > @mode_permission_funcs in checkpatch.pl. > > My question is: how this should be fixed? Why do you think this is a false-positive? > From what I can say, it's a false positive so the correct way to fix it is to > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+". Huh? Provide a patch to show what you are suggesting please. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Resolving "false positive" error message from checkpatch.pl 2017-02-03 10:27 ` Greg KH @ 2017-02-03 12:10 ` Slawomir Stepien 2017-02-03 12:32 ` Greg KH 2017-02-03 12:39 ` Joe Perches 0 siblings, 2 replies; 6+ messages in thread From: Slawomir Stepien @ 2017-02-03 12:10 UTC (permalink / raw) To: Greg KH; +Cc: apw, joe, linux-iio, devel On Feb 03, 2017 11:27, Greg KH wrote: > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote: > > Hi all > > > > There is a "false positive" error reported by checkpatch.pl: > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272: > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO, > > + ade7759_read_8bit, > > + ade7759_write_8bit, > > + ADE7759_CH1OS); > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276: > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO, > > + ade7759_read_8bit, > > + ade7759_write_8bit, > > + ADE7759_CH2OS); > > > > The same for the file drivers/staging/iio/meter/ade7753.c. > > > > We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from > > @mode_permission_funcs in checkpatch.pl. > > > > My question is: how this should be fixed? > > Why do you think this is a false-positive? Because the 1st arg of that macro is not supposed to be a permissions flags. > > From what I can say, it's a false positive so the correct way to fix it is to > > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+". > > Huh? > > Provide a patch to show what you are suggesting please. diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 8e96af53611c..0a1c6ec86caa 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -525,7 +525,7 @@ our @mode_permission_funcs = ( ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2], ["proc_create(?:_data|)", 2], ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2], - ["IIO_DEV_ATTR_[A-Z_]+", 1], + ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1], ["SENSOR_(?:DEVICE_|)ATTR_2", 2], ["SENSOR_TEMPLATE(?:_2|)", 3], ["__ATTR", 2], -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] Resolving "false positive" error message from checkpatch.pl 2017-02-03 12:10 ` Slawomir Stepien @ 2017-02-03 12:32 ` Greg KH 2017-02-03 12:39 ` Joe Perches 1 sibling, 0 replies; 6+ messages in thread From: Greg KH @ 2017-02-03 12:32 UTC (permalink / raw) To: Slawomir Stepien; +Cc: apw, joe, linux-iio, devel On Fri, Feb 03, 2017 at 01:10:20PM +0100, Slawomir Stepien wrote: > On Feb 03, 2017 11:27, Greg KH wrote: > > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote: > > > Hi all > > > > > > There is a "false positive" error reported by checkpatch.pl: > > > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272: > > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO, > > > + ade7759_read_8bit, > > > + ade7759_write_8bit, > > > + ADE7759_CH1OS); > > > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276: > > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO, > > > + ade7759_read_8bit, > > > + ade7759_write_8bit, > > > + ADE7759_CH2OS); > > > > > > The same for the file drivers/staging/iio/meter/ade7753.c. > > > > > > We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from > > > @mode_permission_funcs in checkpatch.pl. > > > > > > My question is: how this should be fixed? > > > > Why do you think this is a false-positive? > > Because the 1st arg of that macro is not supposed to be a permissions flags. Ah. I was thinking that the S_IWUSR needs to be replaced with an octal value :) thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Resolving "false positive" error message from checkpatch.pl 2017-02-03 12:10 ` Slawomir Stepien 2017-02-03 12:32 ` Greg KH @ 2017-02-03 12:39 ` Joe Perches 2017-02-03 13:04 ` Slawomir Stepien 1 sibling, 1 reply; 6+ messages in thread From: Joe Perches @ 2017-02-03 12:39 UTC (permalink / raw) To: Slawomir Stepien, Greg KH; +Cc: apw, linux-iio, devel On Fri, 2017-02-03 at 13:10 +0100, Slawomir Stepien wrote: > On Feb 03, 2017 11:27, Greg KH wrote: > > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote: > > > Hi all > > > > > > There is a "false positive" error reported by checkpatch.pl: > > > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272: > > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO, > > > + ade7759_read_8bit, > > > + ade7759_write_8bit, > > > + ADE7759_CH1OS); > > > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276: > > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO, > > > + ade7759_read_8bit, > > > + ade7759_write_8bit, > > > + ADE7759_CH2OS); > > > > > > The same for the file drivers/staging/iio/meter/ade7753.c. > > > > > > We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from > > > @mode_permission_funcs in checkpatch.pl. > > > > > > My question is: how this should be fixed? > > > > Why do you think this is a false-positive? > > Because the 1st arg of that macro is not supposed to be a permissions flags. > > > > From what I can say, it's a false positive so the correct way to fix it is to > > > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+". > > > > Huh? > > > > Provide a patch to show what you are suggesting please. > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 8e96af53611c..0a1c6ec86caa 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -525,7 +525,7 @@ our @mode_permission_funcs = ( > ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2], > ["proc_create(?:_data|)", 2], > ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2], > - ["IIO_DEV_ATTR_[A-Z_]+", 1], > + ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1], > ["SENSOR_(?:DEVICE_|)ATTR_2", 2], > ["SENSOR_TEMPLATE(?:_2|)", 3], > ["__ATTR", 2], There are many local MODE_DEV_ATTR_<foo> functions that don't have permission in as the first argument. $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq|wc -l 123 Maybe it'd be better to list the ones that do or to change the argument order of the defines and uses so mode _is_ the first argument. $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq| \ while read line ; do git grep -w $line | grep -w define ; done|grep _mode | grep -v "(_mode" drivers/staging/iio/meter/meter.h:#define IIO_DEV_ATTR_CH_OFF(_num, _mode, _show, _store, _addr) \ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQ(_channel, _num, _mode, _show, _store, _addr) \ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQSYMBOL(_channel, _mode, _show, _store, _addr) \ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_OUT_ENABLE(_channel, _mode, _show, _store, _addr) \ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASE(_channel, _num, _mode, _show, _store, _addr) \ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASESYMBOL(_channel, _mode, _show, _store, _addr) \ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PINCONTROL_EN(_channel, _mode, _show, _store, _addr)\ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_channel, _mode, _show, _store, _addr)\ drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_channel, _mode, _show, _store, _addr)\ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] Resolving "false positive" error message from checkpatch.pl 2017-02-03 12:39 ` Joe Perches @ 2017-02-03 13:04 ` Slawomir Stepien 0 siblings, 0 replies; 6+ messages in thread From: Slawomir Stepien @ 2017-02-03 13:04 UTC (permalink / raw) To: Joe Perches; +Cc: Greg KH, apw, linux-iio, devel On Feb 03, 2017 04:39, Joe Perches wrote: > On Fri, 2017-02-03 at 13:10 +0100, Slawomir Stepien wrote: > > On Feb 03, 2017 11:27, Greg KH wrote: > > > On Fri, Feb 03, 2017 at 11:11:35AM +0100, Slawomir Stepien wrote: > > > > Hi all > > > > > > > > There is a "false positive" error reported by checkpatch.pl: > > > > > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > > > #272: FILE: drivers/staging/iio/meter/ade7759.c:272: > > > > +static IIO_DEV_ATTR_CH_OFF(1, S_IWUSR | S_IRUGO, > > > > + ade7759_read_8bit, > > > > + ade7759_write_8bit, > > > > + ADE7759_CH1OS); > > > > > > > > ERROR: Use 4 digit octal (0777) not decimal permissions > > > > #276: FILE: drivers/staging/iio/meter/ade7759.c:276: > > > > +static IIO_DEV_ATTR_CH_OFF(2, S_IWUSR | S_IRUGO, > > > > + ade7759_read_8bit, > > > > + ade7759_write_8bit, > > > > + ADE7759_CH2OS); > > > > > > > > The same for the file drivers/staging/iio/meter/ade7753.c. > > > > > > > > We can see that this macro is matched by the pattern "IIO_DEV_ATTR_[A-Z_]+" from > > > > @mode_permission_funcs in checkpatch.pl. > > > > > > > > My question is: how this should be fixed? > > > > > > Why do you think this is a false-positive? > > > > Because the 1st arg of that macro is not supposed to be a permissions flags. > > > > > > From what I can say, it's a false positive so the correct way to fix it is to > > > > change the matching pattern: "IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+". > > > > > > Huh? > > > > > > Provide a patch to show what you are suggesting please. > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 8e96af53611c..0a1c6ec86caa 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -525,7 +525,7 @@ our @mode_permission_funcs = ( > > ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2], > > ["proc_create(?:_data|)", 2], > > ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2], > > - ["IIO_DEV_ATTR_[A-Z_]+", 1], > > + ["IIO_DEV_ATTR_(?!CH_OFF)[A-Z_]+", 1], > > ["SENSOR_(?:DEVICE_|)ATTR_2", 2], > > ["SENSOR_TEMPLATE(?:_2|)", 3], > > ["__ATTR", 2], > > There are many local MODE_DEV_ATTR_<foo> functions > that don't have permission in as the first argument. Yeah...I just found the ones in the drivers/staging/iio/frequency that you pointed out. > $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq|wc -l > 123 > > Maybe it'd be better to list the ones that do > or to change the argument order of the defines > and uses so mode _is_ the first argument. That is my original question. What would be better here (especially that no we can see that there are few more places where the error will pop)? > $ grep -rPoh --include=*.[ch] "\bIIO_DEV_ATTR_[A-Z_]+\b" *|sort|uniq| \ > while read line ; do git grep -w $line | grep -w define ; done|grep _mode | grep -v "(_mode" > drivers/staging/iio/meter/meter.h:#define IIO_DEV_ATTR_CH_OFF(_num, _mode, _show, _store, _addr) \ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQ(_channel, _num, _mode, _show, _store, _addr) \ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_FREQSYMBOL(_channel, _mode, _show, _store, _addr) \ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_OUT_ENABLE(_channel, _mode, _show, _store, _addr) \ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASE(_channel, _num, _mode, _show, _store, _addr) \ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PHASESYMBOL(_channel, _mode, _show, _store, _addr) \ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PINCONTROL_EN(_channel, _mode, _show, _store, _addr)\ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_channel, _mode, _show, _store, _addr)\ > drivers/staging/iio/frequency/dds.h:#define IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_channel, _mode, _show, _store, _addr)\ -- Slawomir Stepien ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-03 13:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-03 10:11 [RFC] Resolving "false positive" error message from checkpatch.pl Slawomir Stepien 2017-02-03 10:27 ` Greg KH 2017-02-03 12:10 ` Slawomir Stepien 2017-02-03 12:32 ` Greg KH 2017-02-03 12:39 ` Joe Perches 2017-02-03 13:04 ` Slawomir Stepien
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.