* [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.