Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
I ran a treewide script and changed them all to octal and built a kernel which i am currently running on my machine, I have used DEVICE_ATTR_RO() for 0444's where possible if i dont find any regressions I will post a patch for review. I think i will be testing it at least for a week. On Tue, Jul 24, 2018 at 12:00 AM, Joe Perches wrote: > On Mon, 2018-07-23 at 11:24 -0700, Guenter Roeck wrote: > > There are much more urgent issues to fix there (such as, for example, > > converting the "offending" drivers to the latest API, which would > > magically cause most of the offenders to disappear). > > Perhaps posting a list of desired hwmon changes could help. > > Documentation/hwmon/submitting-patches does not seem to specify > what the "latest API" is nor describe what changes would be > required in older drivers. >
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
I ran a treewide script and changed them all to octal and built a kernel which i am currently running on my machine, I have used DEVICE_ATTR_RO() for 0444's where possible if i dont find any regressions I will post a patch for review. I think i will be testing it at least for a week. On Tue, Jul 24, 2018 at 12:00 AM, Joe Perches wrote: > On Mon, 2018-07-23 at 11:24 -0700, Guenter Roeck wrote: > > There are much more urgent issues to fix there (such as, for example, > > converting the "offending" drivers to the latest API, which would > > magically cause most of the offenders to disappear). > > Perhaps posting a list of desired hwmon changes could help. > > Documentation/hwmon/submitting-patches does not seem to specify > what the "latest API" is nor describe what changes would be > required in older drivers. >
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, 2018-07-23 at 11:24 -0700, Guenter Roeck wrote: > There are much more urgent issues to fix there (such as, for example, > converting the "offending" drivers to the latest API, which would > magically cause most of the offenders to disappear). Perhaps posting a list of desired hwmon changes could help. Documentation/hwmon/submitting-patches does not seem to specify what the "latest API" is nor describe what changes would be required in older drivers.
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, 2018-07-23 at 11:24 -0700, Guenter Roeck wrote: > There are much more urgent issues to fix there (such as, for example, > converting the "offending" drivers to the latest API, which would > magically cause most of the offenders to disappear). Perhaps posting a list of desired hwmon changes could help. Documentation/hwmon/submitting-patches does not seem to specify what the "latest API" is nor describe what changes would be required in older drivers.
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 10:25:10AM -0700, Joe Perches wrote: > On Mon, 2018-07-23 at 10:18 -0700, Dmitry Torokhov wrote: > > On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > > > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider > > > > using octal permissions '0200'. > > > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > + .dattr = __ATTR(_field, S_IRUGO, > > > > \ > > > > > > > > Signed-off-by: Harshit Jain > > > > > > This name doesn't match up with the From: line above :( > > > > > > Please fix up and try again. > > > > dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l > > 7605 > > > > We either need to run a tree-wide script or leave this alone. FWIW I am > > perfectly fine with either octals or symbolic names so I do not see > > benefit of doing conversion for code that is not known to be broken. > > About half of those are in one subsystem (drivers/hwmon) > > $ git grep -w S_IRUGO | cut -f1,2 -d'/' | \ > sort | uniq -c | sort -rn | head -10 | cat -n > 1 3846 drivers/hwmon > 2748 drivers/scsi > 3215 drivers/infiniband > 4168 drivers/usb > 5109 drivers/media > 6106 drivers/input > 7102 drivers/platform > 8101 drivers/misc > 9101 drivers/gpu > 10 91 drivers/edac > > The generic reason is octal is readable and S_ is unintelligible. > > https://lkml.org/lkml/2016/8/2/1945 > That is Linus' opinion, but not mine. Granted, my opinion doesn't count much, but enough that I won't waste my time updating the hwmon subsystem to match the new rules. Rules change all the time, and I am quite sure checkpatch will find a lot of stuff to complain about (again) in the hwmon subsystem, after my last spout of cleanup a couple of years ago. There are much more urgent issues to fix there (such as, for example, converting the "offending" drivers to the latest API, which would magically cause most of the offenders to disappear). No one but me cares about those, so bothering about the use of S_ in hwmon seems to be quite pointless. Except, of course, it is quite useful to have something to point to as bad citizen. Guenter
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 10:25:10AM -0700, Joe Perches wrote: > On Mon, 2018-07-23 at 10:18 -0700, Dmitry Torokhov wrote: > > On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > > > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider > > > > using octal permissions '0200'. > > > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > + .dattr = __ATTR(_field, S_IRUGO, > > > > \ > > > > > > > > Signed-off-by: Harshit Jain > > > > > > This name doesn't match up with the From: line above :( > > > > > > Please fix up and try again. > > > > dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l > > 7605 > > > > We either need to run a tree-wide script or leave this alone. FWIW I am > > perfectly fine with either octals or symbolic names so I do not see > > benefit of doing conversion for code that is not known to be broken. > > About half of those are in one subsystem (drivers/hwmon) > > $ git grep -w S_IRUGO | cut -f1,2 -d'/' | \ > sort | uniq -c | sort -rn | head -10 | cat -n > 1 3846 drivers/hwmon > 2748 drivers/scsi > 3215 drivers/infiniband > 4168 drivers/usb > 5109 drivers/media > 6106 drivers/input > 7102 drivers/platform > 8101 drivers/misc > 9101 drivers/gpu > 10 91 drivers/edac > > The generic reason is octal is readable and S_ is unintelligible. > > https://lkml.org/lkml/2016/8/2/1945 > That is Linus' opinion, but not mine. Granted, my opinion doesn't count much, but enough that I won't waste my time updating the hwmon subsystem to match the new rules. Rules change all the time, and I am quite sure checkpatch will find a lot of stuff to complain about (again) in the hwmon subsystem, after my last spout of cleanup a couple of years ago. There are much more urgent issues to fix there (such as, for example, converting the "offending" drivers to the latest API, which would magically cause most of the offenders to disappear). No one but me cares about those, so bothering about the use of S_ in hwmon seems to be quite pointless. Except, of course, it is quite useful to have something to point to as bad citizen. Guenter
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 10:25:10AM -0700, Joe Perches wrote: > On Mon, 2018-07-23 at 10:18 -0700, Dmitry Torokhov wrote: > > On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > > > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider > > > > using octal permissions '0200'. > > > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > + .dattr = __ATTR(_field, S_IRUGO, > > > > \ > > > > > > > > Signed-off-by: Harshit Jain > > > > > > This name doesn't match up with the From: line above :( > > > > > > Please fix up and try again. > > > > dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l > > 7605 > > > > We either need to run a tree-wide script or leave this alone. FWIW I am > > perfectly fine with either octals or symbolic names so I do not see > > benefit of doing conversion for code that is not known to be broken. > > About half of those are in one subsystem (drivers/hwmon) > > $ git grep -w S_IRUGO | cut -f1,2 -d'/' | \ > sort | uniq -c | sort -rn | head -10 | cat -n > 1 3846 drivers/hwmon > 2748 drivers/scsi > 3215 drivers/infiniband > 4168 drivers/usb > 5109 drivers/media > 6106 drivers/input > 7102 drivers/platform > 8101 drivers/misc > 9101 drivers/gpu > 10 91 drivers/edac > > The generic reason is octal is readable and S_ is unintelligible. > > https://lkml.org/lkml/2016/8/2/1945 Yes, but please also read this one as well: https://lkml.org/lkml/2016/8/2/2062 Thanks. -- Dmitry
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 10:25:10AM -0700, Joe Perches wrote: > On Mon, 2018-07-23 at 10:18 -0700, Dmitry Torokhov wrote: > > On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > > > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider > > > > using octal permissions '0200'. > > > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider > > > > using octal permissions '0444'. > > > > + .dattr = __ATTR(_field, S_IRUGO, > > > > \ > > > > > > > > Signed-off-by: Harshit Jain > > > > > > This name doesn't match up with the From: line above :( > > > > > > Please fix up and try again. > > > > dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l > > 7605 > > > > We either need to run a tree-wide script or leave this alone. FWIW I am > > perfectly fine with either octals or symbolic names so I do not see > > benefit of doing conversion for code that is not known to be broken. > > About half of those are in one subsystem (drivers/hwmon) > > $ git grep -w S_IRUGO | cut -f1,2 -d'/' | \ > sort | uniq -c | sort -rn | head -10 | cat -n > 1 3846 drivers/hwmon > 2748 drivers/scsi > 3215 drivers/infiniband > 4168 drivers/usb > 5109 drivers/media > 6106 drivers/input > 7102 drivers/platform > 8101 drivers/misc > 9101 drivers/gpu > 10 91 drivers/edac > > The generic reason is octal is readable and S_ is unintelligible. > > https://lkml.org/lkml/2016/8/2/1945 Yes, but please also read this one as well: https://lkml.org/lkml/2016/8/2/2062 Thanks. -- Dmitry
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, 2018-07-23 at 10:18 -0700, Dmitry Torokhov wrote: > On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > > octal permissions '0444'. > > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > > > octal permissions '0200'. > > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > > octal permissions '0444'. > > > + .dattr = __ATTR(_field, S_IRUGO,\ > > > > > > Signed-off-by: Harshit Jain > > > > This name doesn't match up with the From: line above :( > > > > Please fix up and try again. > > dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l > 7605 > > We either need to run a tree-wide script or leave this alone. FWIW I am > perfectly fine with either octals or symbolic names so I do not see > benefit of doing conversion for code that is not known to be broken. About half of those are in one subsystem (drivers/hwmon) $ git grep -w S_IRUGO | cut -f1,2 -d'/' | \ sort | uniq -c | sort -rn | head -10 | cat -n 1 3846 drivers/hwmon 2 748 drivers/scsi 3 215 drivers/infiniband 4 168 drivers/usb 5 109 drivers/media 6 106 drivers/input 7 102 drivers/platform 8 101 drivers/misc 9 101 drivers/gpu 10 91 drivers/edac The generic reason is octal is readable and S_ is unintelligible. https://lkml.org/lkml/2016/8/2/1945
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, 2018-07-23 at 10:18 -0700, Dmitry Torokhov wrote: > On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > > octal permissions '0444'. > > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > > > octal permissions '0200'. > > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > > octal permissions '0444'. > > > + .dattr = __ATTR(_field, S_IRUGO,\ > > > > > > Signed-off-by: Harshit Jain > > > > This name doesn't match up with the From: line above :( > > > > Please fix up and try again. > > dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l > 7605 > > We either need to run a tree-wide script or leave this alone. FWIW I am > perfectly fine with either octals or symbolic names so I do not see > benefit of doing conversion for code that is not known to be broken. About half of those are in one subsystem (drivers/hwmon) $ git grep -w S_IRUGO | cut -f1,2 -d'/' | \ sort | uniq -c | sort -rn | head -10 | cat -n 1 3846 drivers/hwmon 2 748 drivers/scsi 3 215 drivers/infiniband 4 168 drivers/usb 5 109 drivers/media 6 106 drivers/input 7 102 drivers/platform 8 101 drivers/misc 9 101 drivers/gpu 10 91 drivers/edac The generic reason is octal is readable and S_ is unintelligible. https://lkml.org/lkml/2016/8/2/1945
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > octal permissions '0444'. > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > > octal permissions '0200'. > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > octal permissions '0444'. > > + .dattr = __ATTR(_field, S_IRUGO,\ > > > > Signed-off-by: Harshit Jain > > This name doesn't match up with the From: line above :( > > Please fix up and try again. dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l 7605 We either need to run a tree-wide script or leave this alone. FWIW I am perfectly fine with either octals or symbolic names so I do not see benefit of doing conversion for code that is not known to be broken. Thanks. -- Dmitry
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote: > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > octal permissions '0444'. > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > > octal permissions '0200'. > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > > octal permissions '0444'. > > + .dattr = __ATTR(_field, S_IRUGO,\ > > > > Signed-off-by: Harshit Jain > > This name doesn't match up with the From: line above :( > > Please fix up and try again. dtor@dtor-ws:~/kernel/linux-next$ git grep S_IRU | wc -l 7605 We either need to run a tree-wide script or leave this alone. FWIW I am perfectly fine with either octals or symbolic names so I do not see benefit of doing conversion for code that is not known to be broken. Thanks. -- Dmitry
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > octal permissions '0200'. > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > + .dattr = __ATTR(_field, S_IRUGO,\ > > Signed-off-by: Harshit Jain This name doesn't match up with the From: line above :( Please fix up and try again. thanks, greg k-h
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote: > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > octal permissions '0200'. > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > + .dattr = __ATTR(_field, S_IRUGO,\ > > Signed-off-by: Harshit Jain This name doesn't match up with the From: line above :( Please fix up and try again. thanks, greg k-h
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Sun, Jul 22, 2018 at 12:39:45AM +0530, dev-harsh1998 wrote: > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > octal permissions '0200'. > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > + .dattr = __ATTR(_field, S_IRUGO,\ > > Signed-off-by: dev-harsh1998 You need to use your real name when creating Linux kernel patches. thanks, greg k-h
Re: [PATCH 05/11] touchscreen: elants: Use octal permissions
On Sun, Jul 22, 2018 at 12:39:45AM +0530, dev-harsh1998 wrote: > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL); > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using > octal permissions '0200'. > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw) > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using > octal permissions '0444'. > + .dattr = __ATTR(_field, S_IRUGO,\ > > Signed-off-by: dev-harsh1998 You need to use your real name when creating Linux kernel patches. thanks, greg k-h
