Re: [Outreachy kernel] Re: [PATCH v2] Staging: gasket: Replaces symbolic permissions

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Sat, Feb 23, 2019 at 08:56:41AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Feb 23, 2019 at 11:22:07AM +0530, Bhanusree Pola wrote:
> > Replaces Symbolic Permissions with Octal Permission which solves the
> > checkpatch.pl warning:
> > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
> > 
> > Signed-off-by: Bhanusree Pola <bhanusreemahesh@gmail.com>
> > ---
> > 
> > v2: Subject line modified
> > 
> >  drivers/staging/gasket/gasket_sysfs.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/gasket/gasket_sysfs.h b/drivers/staging/gasket/gasket_sysfs.h
> > index 151e8edd28ea..60590ea4b760 100644
> > --- a/drivers/staging/gasket/gasket_sysfs.h
> > +++ b/drivers/staging/gasket/gasket_sysfs.h
> > @@ -40,7 +40,7 @@
> >   */
> >  #define GASKET_END_OF_ATTR_ARRAY                                               \
> >  	{                                                                      \
> > -		.attr = __ATTR(GASKET_ARRAY_END_TOKEN, S_IRUGO, NULL, NULL),   \
> > +		.attr = __ATTR(GASKET_ARRAY_END_TOKEN, 0444, NULL, NULL),   \
> 
> The trailing "\" is now messed up :(
> 
> But, as I have said before when people try to fix this up, this whole
> macro is not needed at all.  Please just remove it and use the proper
> logic in the code.
> 
> >  		.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),          \
> 
> Trailing \ is not aligned here either.
> 
> Also, this should be using __ATTR_RO(), right?

Really, all of the sysfs stuff in this driver needs to be fixed up
"properly".  I think it's even a TODO item, but that might take a lot
more work than just a simple "coding style" cleanup patch would entail.
I would recommend just leaving this mess alone for now.

thanks,

greg k-h