On Fri, 3 Feb 2017, 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. OK, finally received. I think that Guenter's solution is a good one, because the rule-writer knows better than Coccinelle what people find to be intelligible. julia > > > 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? > > $ git log -1 f90774e1fd2700d > commit f90774e1fd2700de4a6e0d62866d34a26c544bd0 > Author: Joe Perches <joe@xxxxxxxxxxx> > Date: Tue Oct 11 13:51:47 2016 -0700 > > checkpatch: look for symbolic permissions and suggest octal instead > > S_<FOO> uses should be avoided where octal is more intelligible. > > Linus didst say: > > : It's *much* easier to parse and understand the octal numbers, while the > : symbolic macro names are just random line noise and hard as hell to > : understand. You really have to think about it. > : > : So we should rather go the other way: convert existing bad symbolic > : permission bit macro use to just use the octal numbers. > : > : The symbolic names are good for the *other* bits (ie sticky bit, and the > : inode mode _type_ numbers etc), but for the permission bits, the symbolic > : names are just insane crap. Nobody sane should ever use them. Not in the > : kernel, not in user space. > (http://lkml.kernel.org/r/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@xxxxxxxxxxxxxx) > > Link: http://lkml.kernel.org/r/7232ef011d05a92f4caa86a5e9830d87966a2eaf.1470180926.git.joe@xxxxxxxxxxx > Signed-off-by: Joe Perches <joe@xxxxxxxxxxx> > Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
