* [PATCH script] hwmon: Use octal not symbolic permissions
@ 2018-03-26 20:28 Joe Perches
2018-03-27 6:33 ` Guenter Roeck
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2018-03-26 20:28 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
drivers/hwmon is the most frequent user of symbolic permissions
like S_IRUGO in the kernel tree.
$ git grep -w -P "S_[A-Z]{5,5}" | \
cut -f1 -d: | cut -f1-2 -d"/" | sed -r 's/[A-Za-z0-9_-]+\.[ch]$//' | \
sort | uniq -c | sort -rn | head
3862 drivers/hwmon
814 drivers/scsi
763 drivers/net
242 drivers/infiniband
184 drivers/staging
181 drivers/usb
158 fs/proc
150 fs/xfs
148 fs/
142 drivers/misc
But using octal and not symbolic permissions is preferred by many
as it can be more readable.
https://lkml.org/lkml/2016/8/2/1945
Rather than converting these piecemeal, perhaps just do them all
at once via a trivial script like the below:
$ git grep -w -P --name-only "S_[A-Z]{5,5}" drivers/hwmon | \
xargs ./scripts/checkpatch.pl -f --types=symbolic_perms --fix-inplace
$ git grep -w -P --name-only "S_[A-Z]{5,5}" drivers/hwmon | \
xargs ./scripts/checkpatch.pl -f --types=symbolic_perms --fix-inplace
It's run twice because checkpatch only does 1 conversion per line
and there are some multiple instance lines.
This currently results in a 669 KB patch which is too large
to post but can be easily generated when appropriate.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
2018-03-26 20:28 [PATCH script] hwmon: Use octal not symbolic permissions Joe Perches
@ 2018-03-27 6:33 ` Guenter Roeck
2018-03-27 6:52 ` Joe Perches
2018-03-27 7:35 ` Joe Perches
0 siblings, 2 replies; 8+ messages in thread
From: Guenter Roeck @ 2018-03-27 6:33 UTC (permalink / raw)
To: Joe Perches, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
On 03/26/2018 01:28 PM, Joe Perches wrote:
> drivers/hwmon is the most frequent user of symbolic permissions
> like S_IRUGO in the kernel tree.
>
> $ git grep -w -P "S_[A-Z]{5,5}" | \
> cut -f1 -d: | cut -f1-2 -d"/" | sed -r 's/[A-Za-z0-9_-]+\.[ch]$//' | \
> sort | uniq -c | sort -rn | head
> 3862 drivers/hwmon
> 814 drivers/scsi
> 763 drivers/net
> 242 drivers/infiniband
> 184 drivers/staging
> 181 drivers/usb
> 158 fs/proc
> 150 fs/xfs
> 148 fs/
> 142 drivers/misc
>
> But using octal and not symbolic permissions is preferred by many
> as it can be more readable.
>
> https://lkml.org/lkml/2016/8/2/1945
>
> Rather than converting these piecemeal, perhaps just do them all
> at once via a trivial script like the below:
>
> $ git grep -w -P --name-only "S_[A-Z]{5,5}" drivers/hwmon | \
> xargs ./scripts/checkpatch.pl -f --types=symbolic_perms --fix-inplace
> $ git grep -w -P --name-only "S_[A-Z]{5,5}" drivers/hwmon | \
> xargs ./scripts/checkpatch.pl -f --types=symbolic_perms --fix-inplace
>
> It's run twice because checkpatch only does 1 conversion per line
> and there are some multiple instance lines.
>
> This currently results in a 669 KB patch which is too large
> to post but can be easily generated when appropriate.
I have something similar using coccinelle, which has the added benefit
of also adjusting multi-line alignments. But then I am hesitant to pull
it in because I don't really see the point. A more intelligent approach
would be to convert hwmon drivers to the latest API, and/or to introduce
more intelligent macros such as SENSOR_DEVICE_ATTR_{RO,RW,WO}. But that
would require active work as well as reviewers, and especially the latter
is extremely difficult if not impossible to find for the hwmon subsystem.
Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
that is maybe not entirely surprising, and I think we are good as we are.
I am happy to accept patches updating permissions as other changes are made
to a file, but I don't see a pressing need to change all files just to make
statistics happy (and backports more difficult).
Guenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
2018-03-27 6:33 ` Guenter Roeck
@ 2018-03-27 6:52 ` Joe Perches
2018-03-27 7:35 ` Joe Perches
1 sibling, 0 replies; 8+ messages in thread
From: Joe Perches @ 2018-03-27 6:52 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
> On 03/26/2018 01:28 PM, Joe Perches wrote:
> > drivers/hwmon is the most frequent user of symbolic permissions
> > like S_IRUGO in the kernel tree.
[]
> I have something similar using coccinelle,
Please post the script.
I expect it doesn't work quite the same as checkpatch.
> which has the added benefit
> of also adjusting multi-line alignments.
That's a benefit, but it's makes it difficult to review.
I also have the same patch rewrap alignment post this
automated patch, done with emacs formatting.
This patch is somewhat simpler to review which is why
it's not sent along with this script.
> But then I am hesitant to pull
> it in because I don't really see the point. A more intelligent approach
> would be to convert hwmon drivers to the latest API, and/or to introduce
> more intelligent macros such as SENSOR_DEVICE_ATTR_{RO,RW,WO}.
Another approach would be to separate the const bits
from the non-const bits to reduce data.
> But that
> would require active work as well as reviewers, and especially the latter
> is extremely difficult if not impossible to find for the hwmon subsystem.
[]
> Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
> that is maybe not entirely surprising, and I think we are good as we are.
Maybe true.
> I am happy to accept patches updating permissions as other changes are made
> to a file, but I don't see a pressing need to change all files just to make
> statistics happy (and backports more difficult).
Not sure backports are a real issue.
If it were a real issue, the updated permissions are noise
and shouldn't be accepted for the other changes.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
2018-03-27 6:33 ` Guenter Roeck
2018-03-27 6:52 ` Joe Perches
@ 2018-03-27 7:35 ` Joe Perches
2018-03-27 10:28 ` Guenter Roeck
1 sibling, 1 reply; 8+ messages in thread
From: Joe Perches @ 2018-03-27 7:35 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
> Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
fyi: It's marked Maintained in MAINTAINERS
HARDWARE MONITORING
M: Jean Delvare <jdelvare@suse.com>
M: Guenter Roeck <linux@roeck-us.net>
L: linux-hwmon@vger.kernel.org
W: http://hwmon.wiki.kernel.org/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
S: Maintained
F: Documentation/hwmon/
F: drivers/hwmon/
F: include/linux/hwmon*.h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
2018-03-27 7:35 ` Joe Perches
@ 2018-03-27 10:28 ` Guenter Roeck
2018-03-27 11:48 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2018-03-27 10:28 UTC (permalink / raw)
To: Joe Perches, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
On 03/27/2018 12:35 AM, Joe Perches wrote:
> On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
>> Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
>
> fyi: It's marked Maintained in MAINTAINERS
>
I did not say that it was not maintained, and I am aware of the information
in the MAINTAINERS file, thanks.
Guenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
2018-03-27 10:28 ` Guenter Roeck
@ 2018-03-27 11:48 ` Joe Perches
2018-03-27 15:44 ` Guenter Roeck
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2018-03-27 11:48 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
On Tue, 2018-03-27 at 03:28 -0700, Guenter Roeck wrote:
> On 03/27/2018 12:35 AM, Joe Perches wrote:
> > On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
> > > Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
> >
> > fyi: It's marked Maintained in MAINTAINERS
> >
>
> I did not say that it was not maintained, and I am aware of the information
> in the MAINTAINERS file, thanks.
As am I.
I do agree that hwmon is obscure. Almost everything about
anything is obscure to those that don't fully understand it.
I am unaware though of any use of "obsolete" in drivers/hwmon/
or in Documentation/ about hwmon or the wiki/
In what sense do you mean hwmon is obsolete?
And the cocci script?
Anything later than:
https://systeme.lip6.fr/pipermail/cocci/2016-December/003887.html
https://github.com/groeck/coccinelle-patches/tree/master/hwmon
?
btw: I think the perl scripts I posted for DEV_ATTR_<RW|RO|WO>
transforms are simpler and less prone to tool versioning
issues.
https://lkml.org/lkml/2017/12/22/844
It'd be fairly simple to add SENSOR_ support.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
2018-03-27 11:48 ` Joe Perches
@ 2018-03-27 15:44 ` Guenter Roeck
2018-03-27 16:52 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2018-03-27 15:44 UTC (permalink / raw)
To: Joe Perches, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
On 03/27/2018 04:48 AM, Joe Perches wrote:
> On Tue, 2018-03-27 at 03:28 -0700, Guenter Roeck wrote:
>> On 03/27/2018 12:35 AM, Joe Perches wrote:
>>> On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
>>>> Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
>>>
>>> fyi: It's marked Maintained in MAINTAINERS
>>>
>>
>> I did not say that it was not maintained, and I am aware of the information
>> in the MAINTAINERS file, thanks.
>
> As am I.
>
> I do agree that hwmon is obscure. Almost everything about
> anything is obscure to those that don't fully understand it.
>
> I am unaware though of any use of "obsolete" in drivers/hwmon/
> or in Documentation/ about hwmon or the wiki/
>
> In what sense do you mean hwmon is obsolete?
>
_I_ did not say it was obsolete. It was described as obsolete by a maintainer
of a different subsystem at a conference a couple of years ago. If I wasn't too
lazy, I could try to find the presentation, but I don't see the point.
> And the cocci script?
>
> Anything later than:
> https://systeme.lip6.fr/pipermail/cocci/2016-December/003887.html
> https://github.com/groeck/coccinelle-patches/tree/master/hwmon
> ?
>
I don't think I ever published it. If I recall correctly, it required
some cleanup patches to be applied first, but then I gave up on the idea
because I didn't want to proceed without code reviews.
> btw: I think the perl scripts I posted for DEV_ATTR_<RW|RO|WO>
> transforms are simpler and less prone to tool versioning
> issues.
>
> https://lkml.org/lkml/2017/12/22/844
>
> It'd be fairly simple to add SENSOR_ support.
>
I'll keep it in mind.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH script] hwmon: Use octal not symbolic permissions
2018-03-27 15:44 ` Guenter Roeck
@ 2018-03-27 16:52 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2018-03-27 16:52 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare
Cc: linux-hwmon, linux-kernel, linuxppc-dev, linux-arm-kernel, patches
On Tue, 2018-03-27 at 08:44 -0700, Guenter Roeck wrote:
> On 03/27/2018 04:48 AM, Joe Perches wrote:
> > On Tue, 2018-03-27 at 03:28 -0700, Guenter Roeck wrote:
> > > On 03/27/2018 12:35 AM, Joe Perches wrote:
> > > > On Mon, 2018-03-26 at 23:33 -0700, Guenter Roeck wrote:
> > > > > Since the hwmon subsystem has been labeled as both "obsolete" and "obscure",
[]
> > In what sense do you mean hwmon is obsolete?
[]
> _I_ did not say it was obsolete. It was described as obsolete by a maintainer
> of a different subsystem at a conference a couple of years ago. If I wasn't too
> lazy, I could try to find the presentation, but I don't see the point.
Then it seems you were just being obscure on purpose.
You wrote obsolete without context.
cheers, Joe
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-27 16:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-26 20:28 [PATCH script] hwmon: Use octal not symbolic permissions Joe Perches
2018-03-27 6:33 ` Guenter Roeck
2018-03-27 6:52 ` Joe Perches
2018-03-27 7:35 ` Joe Perches
2018-03-27 10:28 ` Guenter Roeck
2018-03-27 11:48 ` Joe Perches
2018-03-27 15:44 ` Guenter Roeck
2018-03-27 16:52 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).