Re: Staging: speakup - syle fix permissions to octal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/04/2017 06:29 AM, Julia Lawall wrote:


On Sat, 4 Feb 2017, Guenter Roeck wrote:

On 02/03/2017 11:27 PM, Joe Perches wrote:
(adding Julia Lawall)

On Fri, 2017-02-03 at 20:44 -0800, Guenter Roeck wrote:
On Sat, Jan 28, 2017 at 07:05:09PM +1300, Derek Robson wrote:
A style fix across whole driver.
changed permissions to octal style, found using checkpatch

Signed-off-by: Derek Robson <robsonde@xxxxxxxxx>

FWIW, I think changes like this are best done using coccinelle.

I think checkpatch does it reasonably well.

Julia?  Can coccinelle do this?

I believe cocinelle doesn't handle the substitution
and octal addition very well when multiple flags
are used.


Why not ? Seems to be quite simple. One just has to list all the variants
being used in the rule.

That ensures that the results can be reproduced and are well defined.
As it is, someone will have to check each line of your patches to ensure
that the conversion is correct.

It would also ensure (hopefully) that we don't end up with constructs
such as

-#define USER_R (S_IFREG|S_IRUGO)
-#define USER_W (S_IFREG|S_IWUGO)
+#define USER_R (S_IFREG|0444)
+#define USER_W (S_IFREG|0666)

which really defeat the purpose of the whole exercise.

Why do you think mixing file specific attributes
with octal permissions is a bad thing?


Just an assumption. My bad. Ultimately, what I think doesn't really
matter, though - because what I think is that the whole "use octals"
is a bad idea to start with.

I don't think I have received yet the message that this is referring to.
But I don't see a problem for Coccinelle a priori.  If there are things
that need to be added together, as long as they are explicit constants,
that can be done in python or ocaml.


Something like

@@
@@

(
- S_IFREG | S_IRUGO | S_IWUGO
+ S_IFREG | 0666
|
- S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
+ 0644
|
- S_IRUGO|S_IWUSR
+ 0644
|
- S_IWUSR|S_IRUGO
+ 0644
|
- S_IRUGO|S_IWUGO
+ 0666
|
- S_IWUGO|S_IRUGO
+ 0666
|
- S_IRUGO
+ 0444
|
- S_IWUGO
+ 0222
|
- S_IWUSR
+ 0200
)

Odd is that the S_IFREG rule seems to be needed to catch "S_IFREG | S_IRUGO | S_IWUGO",
but probably I am missing something as usual ;-).

Guenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux