[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

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

[Wentao Cai]

This patch fixes the checkpatch.pl warning:
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.

Signed-off-by: Wentao Cai <etsai042@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 151e8edd28ea..708ee43175ba 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                                   \
 	}
 
-- 
2.17.1

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

[Greg Kroah-Hartman]

On Fri, Mar 01, 2019 at 04:12:38PM +0800, Wentao Cai wrote:
> This patch fixes the checkpatch.pl warning:
> WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
> 
> Signed-off-by: Wentao Cai <etsai042@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 151e8edd28ea..708ee43175ba 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 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.

thanks,

greg k-h

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

[etsai042]

[-- Attachment #1.1: Type: text/plain, Size: 2042 bytes --]

Sorry Greg, I'll check the archive first next time, before I send my 
patches.

Wentao Cai

在 2019年3月1日星期五 UTC+8下午4:19:07，gregkh写道：
>
> On Fri, Mar 01, 2019 at 04:12:38PM +0800, Wentao Cai wrote: 
> > This patch fixes the checkpatch.pl warning: 
> > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider 
> using octal permissions '0444'. 
> > 
> > Signed-off-by: Wentao Cai <etsa...@gmail.com <javascript:>> 
> > --- 
> >  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..708ee43175ba 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 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. 
>
> thanks, 
>
> greg k-h 
>

[-- Attachment #1.2: Type: text/html, Size: 3213 bytes --]

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

[Wentao Cai]

[-- Attachment #1.1: Type: text/plain, Size: 1538 bytes --]



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?

Thanks,

Wentao

 

> thanks, 
>
> greg k-h 
>

[-- Attachment #1.2: Type: text/html, Size: 2579 bytes --]

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

[Himadri Pandya]

Resolve checkpatch warning for using symbolic permissions by replacing
them with octal permissions.

Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
 drivers/staging/gasket/gasket_sysfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/gasket/gasket_sysfs.h b/drivers/staging/gasket/gasket_sysfs.h
index 1d0eed66a7f4..28dc422d388c 100644
--- a/drivers/staging/gasket/gasket_sysfs.h
+++ b/drivers/staging/gasket/gasket_sysfs.h
@@ -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                                   \
 	}
 
-- 
2.17.1

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

[Greg KH]

On Wed, Apr 10, 2019 at 03:58:48PM +0530, Himadri Pandya wrote:
> Resolve checkpatch warning for using symbolic permissions by replacing
> them with octal permissions.
> 
> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> ---
>  drivers/staging/gasket/gasket_sysfs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/gasket/gasket_sysfs.h b/drivers/staging/gasket/gasket_sysfs.h
> index 1d0eed66a7f4..28dc422d388c 100644
> --- a/drivers/staging/gasket/gasket_sysfs.h
> +++ b/drivers/staging/gasket/gasket_sysfs.h
> @@ -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                                   \
>  	}

This has been rejected many times in the past as the real solution is to
use __ATTR_RO here.

thanks,

greg k-h