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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
