Re: [Outreachy kernel] Re: [PATCH] Staging: gasket: Replace symbolic permissions with octal permissions

[Greg KH <greg@kroah.com>]

On Wed, Mar 13, 2019 at 01:50:33AM -0700, Wentao Cai wrote:
> 
> 
> On Friday, March 1, 2019 at 12:19:07 AM UTC-8, gregkh wrote:
> >
> >
> > This is fun, it keeps coming up and no one looks in the archives to see 
> > why I keep rejecting it :) 
> >
> > This should be __ATTR_NULL(), but really, the whole macro can be deleted. 
> >
> > >                  .data.attr_type = 0,                                   
> >         \ 
> > >          } 
> > >   
> > > @@ -75,7 +75,7 @@ struct gasket_sysfs_attribute { 
> > >   
> > >  #define GASKET_SYSFS_RO(_name, _show_function, _attr_type)             
> >         \ 
> > >          {                                                               
> >        \ 
> > > -                .attr = __ATTR(_name, S_IRUGO, _show_function, NULL),   
> >        \ 
> > > +                .attr = __ATTR(_name, 0444, _show_function, NULL),     
> >         \ 
> >
> > __ATTR_RO() please. 
> >
> > Hi. I just submit a patch to fix the first __ATTR_NULL, but I have some 
> question about the latter one.
> I understand it is better to use read only attribute here, but by changing 
> __ATTR to __ATTR_RO, since the new macro does not need the _show_function  
> parameter:
> 
> > (linux/sysfs.h, line 115)
> > #define __ATTR_RO(_name) { \
> > .attr = { .name = __stringify(_name), .mode = 0444 }, \
> > .show = _name##_show, \
> > }
> 
> it seems to me that the second parameter in this macro should be deleted, 
> which means all other file which calls this macro should be revise. 
> I think it's a big code change, and don't know whether it's suitable to do 
> this. Could you give me some advice?

Yes, it is a big code change, and if you are not comfortable with it, I
would not recommend it at this point in time.

This is also why no one has done it yet :)

thanks,

greg k-h