hwmon: Use octal not symbolic permissions

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

[]

Please post the script.
I expect it doesn't work quite the same as checkpatch.

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.

Another approach would be to separate the const bits from the non-const bits to reduce data.

[]

Maybe true.

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.

fyi: It's marked Maintained in MAINTAINERS

HARDWARE MONITORING
M: Jean Delvare <[email protected]>
M: Guenter Roeck <[email protected]>
L: linux-hwmon at 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

I did not say that it was not maintained, and I am aware of the information in the MAINTAINERS file, thanks.

Guenter

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.

_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.

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.

I'll keep it in mind.

Thanks,
Guenter

[]

[]

Then it seems you were just being obscure on purpose. You wrote obsolete without context.

cheers, Joe