[PATCH] staging: gasket: Replace symbolic permissions with octal permissions

[PATCH] staging: gasket: Replace symbolic permissions with octal permissions

[Mamta Shukla]

Use octal permissions in place of symbolic permissions to fix checkpatch
warning.
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider
using octal permissions '0444'.

Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
---
 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 f32eaf8..8b563d2 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),   \
 		.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),          \
 		.data.attr_type = _attr_type                                   \
 	}
 
-- 
1.9.1

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

[Himanshu Jha]

On Sat, Oct 06, 2018 at 08:03:16PM +0530, Mamta Shukla wrote:
> Use octal permissions in place of symbolic permissions to fix checkpatch
> warning.
> WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider
> using octal permissions '0444'.
> 
> Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>

Reviewed-by: Himanshu Jha <himanshujha199640@gmail.com>

In the meantime take a look here:
https://lkml.org/lkml/2016/8/2/1945


> ---
>  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 f32eaf8..8b563d2 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),   \
>  		.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),          \
>  		.data.attr_type = _attr_type                                   \
>  	}
>  
> -- 
> 1.9.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181006143316.GA23864%40armorer.
> For more options, visit https://groups.google.com/d/optout.

-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Re: [PATCH] staging: gasket: Replace symbolic permissions with octal permissions

[Greg KH]

On Sat, Oct 06, 2018 at 08:03:16PM +0530, Mamta Shukla wrote:
> Use octal permissions in place of symbolic permissions to fix checkpatch
> warning.
> WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider
> using octal permissions '0444'.
> 
> Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> ---
>  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 f32eaf8..8b563d2 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),   \

This is very odd, why is this define even needed?

>  		.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),          \

This should use __ATTR_RO() instead, right?

thanks,

greg k-h

Re: [PATCH] staging: gasket: Replace symbolic permissions with octal permissions

[Mamta Shukla]

[-- Attachment #1: Type: text/plain, Size: 1875 bytes --]

On Tue, Oct 9, 2018 at 6:25 PM Greg KH <greg@kroah.com> wrote:

> On Sat, Oct 06, 2018 at 08:03:16PM +0530, Mamta Shukla wrote:
> > Use octal permissions in place of symbolic permissions to fix checkpatch
> > warning.
> > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider
> > using octal permissions '0444'.
> >
> > Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> > ---
> >  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 f32eaf8..8b563d2 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),
>  \
>
> >This is very odd, why is this define even needed?
>

This  gasket_sysfs.h and gasket_sysfs.c utility allows gasket driver
framework to maintain their own set of sysfs entries. This was for
terminating
gasket_sysfs_attr array.
Is this understanding correct?



> >               .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),
>   \
>
> >This should use __ATTR_RO() instead, right?
>

Yes, this should be of Read only type attribute.This driver deals with
'raw' sysfs attributes.
Can I write a different patch for this?



>
> >thanks,
>
> >greg k-h
>


-- 
Mamta Shukla.

[-- Attachment #2: Type: text/html, Size: 3433 bytes --]

Re: [PATCH] staging: gasket: Replace symbolic permissions with octal permissions

[Greg KH]

On Wed, Oct 10, 2018 at 06:40:54PM +0530, Mamta Shukla wrote:
> On Tue, Oct 9, 2018 at 6:25 PM Greg KH <greg@kroah.com> wrote:
> 
> > On Sat, Oct 06, 2018 at 08:03:16PM +0530, Mamta Shukla wrote:
> > > Use octal permissions in place of symbolic permissions to fix checkpatch
> > > warning.
> > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider
> > > using octal permissions '0444'.
> > >
> > > Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> > > ---
> > >  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 f32eaf8..8b563d2 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),
> >  \
> >
> > >This is very odd, why is this define even needed?
> >
> 
> This  gasket_sysfs.h and gasket_sysfs.c utility allows gasket driver
> framework to maintain their own set of sysfs entries. This was for
> terminating
> gasket_sysfs_attr array.
> Is this understanding correct?

That seems correct.  But the code should just be using a NULL attribute
instead, no other subsystem has stuff like this.  So the code should be
changed to not need this.  And really, the permission here is just
pointless anyway...

> > >               .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),
> >   \
> >
> > >This should use __ATTR_RO() instead, right?
> >
> 
> Yes, this should be of Read only type attribute.This driver deals with
> 'raw' sysfs attributes.
> Can I write a different patch for this?

Yes, please just redo this patch and use __ATTR_RO() instead of this
change.

thanks,

greg k-h