* [PATCH v2] Staging: gasket: Replaces symbolic permissions
@ 2019-02-23 5:52 Bhanusree Pola
2019-02-23 7:56 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Bhanusree Pola @ 2019-02-23 5:52 UTC (permalink / raw)
To: outreachy-kernel; +Cc: Greg Kroah-Hartman, Bhanusree Pola
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), \
.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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Staging: gasket: Replaces symbolic permissions
2019-02-23 5:52 [PATCH v2] Staging: gasket: Replaces symbolic permissions Bhanusree Pola
@ 2019-02-23 7:56 ` Greg Kroah-Hartman
2019-02-23 8:42 ` [Outreachy kernel] " Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-23 7:56 UTC (permalink / raw)
To: Bhanusree Pola; +Cc: outreachy-kernel
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?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v2] Staging: gasket: Replaces symbolic permissions
2019-02-23 7:56 ` Greg Kroah-Hartman
@ 2019-02-23 8:42 ` Greg Kroah-Hartman
2019-02-23 9:34 ` Bhanusree Mahesh
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-23 8:42 UTC (permalink / raw)
To: Bhanusree Pola; +Cc: outreachy-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v2] Staging: gasket: Replaces symbolic permissions
2019-02-23 8:42 ` [Outreachy kernel] " Greg Kroah-Hartman
@ 2019-02-23 9:34 ` Bhanusree Mahesh
2019-02-23 9:51 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Bhanusree Mahesh @ 2019-02-23 9:34 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: outreachy-kernel
On Sat, 23 Feb 2019 at 14:12, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> 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.
>
I'm really interested in trying this if you say so. Please give me
some link on this.
I will understand and try to fix them up.
thanks,
Bhanusree Pola.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v2] Staging: gasket: Replaces symbolic permissions
2019-02-23 9:34 ` Bhanusree Mahesh
@ 2019-02-23 9:51 ` Greg Kroah-Hartman
0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-23 9:51 UTC (permalink / raw)
To: Bhanusree Mahesh; +Cc: outreachy-kernel
On Sat, Feb 23, 2019 at 03:04:21PM +0530, Bhanusree Mahesh wrote:
> On Sat, 23 Feb 2019 at 14:12, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > 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.
> >
> I'm really interested in trying this if you say so. Please give me
> some link on this.
> I will understand and try to fix them up.
The last item in the TODO file in this directory is the short summary of
the problem:
- "drivers" should never be dealing with "raw" sysfs calls or
mess around with kobjects at all. The driver core should
handle all of this for you automaically. There should not be a
need for raw attribute macros.
Note, it's not an easy or simple thing to fix up, it requires messing
with how the code interacts with the driver model. I'm not going to say
you shouldn't do it, but I personally don't have the time right now to
mentor anyone to do it. The driver core isn't the easiest thing to
understand in places, sorry.
In the long run, this whole driver is going to be deleted anyway, as it
should be using the UIO api instead. But that's a much more complex and
difficult task as it requires also changing the userspace code that
interacts with this driver (and I have no idea where that is), and would
require access to the hardware involved to make sure it all works (and I
have no idea how to get access to that hardware.)
sorry,
greg k-h
>
> thanks,
>
> Bhanusree Pola.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-23 9:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-23 5:52 [PATCH v2] Staging: gasket: Replaces symbolic permissions Bhanusree Pola
2019-02-23 7:56 ` Greg Kroah-Hartman
2019-02-23 8:42 ` [Outreachy kernel] " Greg Kroah-Hartman
2019-02-23 9:34 ` Bhanusree Mahesh
2019-02-23 9:51 ` Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.