[PATCH 0/7] security: apparmor: apparmorfs.c Checkpatch mods
Make checkpatch modifications to include: Multiple blank lines Blank line after declarations Space after cast WARN_ON vs BUG_ON Align parenthesis Comparison to NULL Line continuations Walt Feasel (7): security: apparmor: apparmorfs.c Multiple blank lines security: apparmor: apparmorfs.c Blank line after declarations security: apparmor: apparmorfs.c Space after cast security: apparmor: apparmorfs.c WARN_ON vs BUG_ON security: apparmor: apparmorfs.c Align parenthesis security: apparmor: apparmorfs.c Comparison to NULL security: apparmor: apparmorfs.c Line continuations security/apparmor/apparmorfs.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) -- Have questions on if the WARN_ON is used properly in patch 4 and also for the comparrison to NULL patch in patch 6 2.1.4
Make style modifications for: CHECK: Alignment should match open parenthesis Signed-off-by: Walt Feasel <waltfeasel@gmail.com> --- Swaps align for over 80 but get whole line this way security/apparmor/apparmorfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index d759c78..c8142cf 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -647,8 +647,7 @@ static struct aa_profile *__next_profile(struct aa_profile *p) if (!list_entry_is_head(p, &parent->base.profiles, base.list)) return p; p = parent; - parent = rcu_dereference_protected(parent->parent, - mutex_is_locked(&parent->ns->lock)); + parent = rcu_dereference_protected(parent->parent, mutex_is_locked(&parent->ns->lock)); } /* is next another profile in the namespace */ -- 2.1.4
On Sat, Nov 26, 2016 at 03:43:18AM -0500, Walt Feasel wrote:
Make checkpatch modifications to include:
Multiple blank lines Blank line after declarations Space after cast WARN_ON vs BUG_ON Align parenthesis Comparison to NULL Line continuations
Walt Feasel (7): security: apparmor: apparmorfs.c Multiple blank lines security: apparmor: apparmorfs.c Blank line after declarations security: apparmor: apparmorfs.c Space after cast security: apparmor: apparmorfs.c WARN_ON vs BUG_ON security: apparmor: apparmorfs.c Align parenthesis security: apparmor: apparmorfs.c Comparison to NULL security: apparmor: apparmorfs.c Line continuations
Why are you sending apparmor patches to the kernelnewbies list and not the maintainers and subsystem list for this code instead? thanks, greg k-h
On Sat, Nov 26, 2016 at 12:05:50PM +0100, Greg KH wrote:
On Sat, Nov 26, 2016 at 03:43:18AM -0500, Walt Feasel wrote:
Make checkpatch modifications to include:
Multiple blank lines Blank line after declarations Space after cast WARN_ON vs BUG_ON Align parenthesis Comparison to NULL Line continuations
Walt Feasel (7): security: apparmor: apparmorfs.c Multiple blank lines security: apparmor: apparmorfs.c Blank line after declarations security: apparmor: apparmorfs.c Space after cast security: apparmor: apparmorfs.c WARN_ON vs BUG_ON security: apparmor: apparmorfs.c Align parenthesis security: apparmor: apparmorfs.c Comparison to NULL security: apparmor: apparmorfs.c Line continuations
Why are you sending apparmor patches to the kernelnewbies list and not the maintainers and subsystem list for this code instead?
thanks,
greg k-h Because as stated in my patch notes I am not sure how to fix certain cases. I have not been able to find reference material other than other patches and discussions for certain cases and have to try and reason why. You wanted me to move out of staging so I have looked to other areas but do not wish to send in ridiculous patches and have me seen as being malicious. Since this is kernelnewbies mailing list isn't that it's purpose or am I move on again? Feel free to skip my post should they trouble you.
On Sat, Nov 26, 2016 at 11:56:17AM -0500, Walt Feasel wrote:
On Sat, Nov 26, 2016 at 12:05:50PM +0100, Greg KH wrote:
On Sat, Nov 26, 2016 at 03:43:18AM -0500, Walt Feasel wrote:
Make checkpatch modifications to include:
Multiple blank lines Blank line after declarations Space after cast WARN_ON vs BUG_ON Align parenthesis Comparison to NULL Line continuations
Walt Feasel (7): security: apparmor: apparmorfs.c Multiple blank lines security: apparmor: apparmorfs.c Blank line after declarations security: apparmor: apparmorfs.c Space after cast security: apparmor: apparmorfs.c WARN_ON vs BUG_ON security: apparmor: apparmorfs.c Align parenthesis security: apparmor: apparmorfs.c Comparison to NULL security: apparmor: apparmorfs.c Line continuations
Why are you sending apparmor patches to the kernelnewbies list and not the maintainers and subsystem list for this code instead?
thanks,
greg k-h Because as stated in my patch notes I am not sure how to fix certain cases. I have not been able to find reference material other than other patches and discussions for certain cases and have to try and reason why. You wanted me to move out of staging so I have looked to other areas but do not wish to send in ridiculous patches and have me seen as being malicious. Since this is kernelnewbies mailing list isn't that it's purpose or am I move on again? Feel free to skip my post should they trouble you.
Just send the patches to the proper people and list that get_maintainer.pl shows you to, that's all, no need to bother kernelnewbies with it. good luck! greg k-h
On Sat, Nov 26, 2016 at 06:19:22PM +0100, Greg KH wrote:
On Sat, Nov 26, 2016 at 11:56:17AM -0500, Walt Feasel wrote:
On Sat, Nov 26, 2016 at 12:05:50PM +0100, Greg KH wrote:
On Sat, Nov 26, 2016 at 03:43:18AM -0500, Walt Feasel wrote:
Make checkpatch modifications to include:
Multiple blank lines Blank line after declarations Space after cast WARN_ON vs BUG_ON Align parenthesis Comparison to NULL Line continuations
Walt Feasel (7): security: apparmor: apparmorfs.c Multiple blank lines security: apparmor: apparmorfs.c Blank line after declarations security: apparmor: apparmorfs.c Space after cast security: apparmor: apparmorfs.c WARN_ON vs BUG_ON security: apparmor: apparmorfs.c Align parenthesis security: apparmor: apparmorfs.c Comparison to NULL security: apparmor: apparmorfs.c Line continuations
Why are you sending apparmor patches to the kernelnewbies list and not the maintainers and subsystem list for this code instead?
thanks,
greg k-h Because as stated in my patch notes I am not sure how to fix certain cases. I have not been able to find reference material other than other patches and discussions for certain cases and have to try and reason why. You wanted me to move out of staging so I have looked to other areas but do not wish to send in ridiculous patches and have me seen as being malicious. Since this is kernelnewbies mailing list isn't that it's purpose or am I move on again? Feel free to skip my post should they trouble you.
Just send the patches to the proper people and list that get_maintainer.pl shows you to, that's all, no need to bother kernelnewbies with it.
good luck!
greg k-h So kernelnewbies is not to be used to learn about how to fix checkpatch type warning?
On Sat, Nov 26, 2016 at 01:02:14PM -0500, Walt Feasel wrote:
On Sat, Nov 26, 2016 at 06:19:22PM +0100, Greg KH wrote:
On Sat, Nov 26, 2016 at 11:56:17AM -0500, Walt Feasel wrote:
On Sat, Nov 26, 2016 at 12:05:50PM +0100, Greg KH wrote:
On Sat, Nov 26, 2016 at 03:43:18AM -0500, Walt Feasel wrote:
Make checkpatch modifications to include:
Multiple blank lines Blank line after declarations Space after cast WARN_ON vs BUG_ON Align parenthesis Comparison to NULL Line continuations
Walt Feasel (7): security: apparmor: apparmorfs.c Multiple blank lines security: apparmor: apparmorfs.c Blank line after declarations security: apparmor: apparmorfs.c Space after cast security: apparmor: apparmorfs.c WARN_ON vs BUG_ON security: apparmor: apparmorfs.c Align parenthesis security: apparmor: apparmorfs.c Comparison to NULL security: apparmor: apparmorfs.c Line continuations
Why are you sending apparmor patches to the kernelnewbies list and not the maintainers and subsystem list for this code instead?
thanks,
greg k-h Because as stated in my patch notes I am not sure how to fix certain cases. I have not been able to find reference material other than other patches and discussions for certain cases and have to try and reason why. You wanted me to move out of staging so I have looked to other areas but do not wish to send in ridiculous patches and have me seen as being malicious. Since this is kernelnewbies mailing list isn't that it's purpose or am I move on again? Feel free to skip my post should they trouble you.
Just send the patches to the proper people and list that get_maintainer.pl shows you to, that's all, no need to bother kernelnewbies with it.
good luck!
greg k-h So kernelnewbies is not to be used to learn about how to fix checkpatch type warning?
Maybe, but really, if you have a well-formed patch, just send it to the correct maintainers, it's up to them to accept it or not, that's their job :) greg k-h
Greg KH <greg@kroah.com> writes:
On Sat, Nov 26, 2016 at 01:02:14PM -0500, Walt Feasel wrote:
So kernelnewbies is not to be used to learn about how to fix checkpatch type warning?
Maybe, but really, if you have a well-formed patch, just send it to the correct maintainers, it's up to them to accept it or not, that's their job :)
And I think the most important reason for doing that is because only they can answer the questions. You need to know what the code does to be able to answer things like "is it OK to replace BUG_ON with WARN_ON here?". Although that is preferable according to checkpatch, it's not necessarily a 1-to-1 replacement. The error path changes, and the existing code is likely not tested or developed with the new path in mind. Nobody(?) in kernelnewbies knows anything about the apparmor code. It's not that we don't want to answer. We just can't. Bjørn
On Nov 26, 2016 1:28 PM, "Bjørn Mork" <bjorn@mork.no> wrote:
Greg KH <greg@kroah.com> writes:
On Sat, Nov 26, 2016 at 01:02:14PM -0500, Walt Feasel wrote:
So kernelnewbies is not to be used to learn about how to fix checkpatch type warning?
Maybe, but really, if you have a well-formed patch, just send it to the correct maintainers, it's up to them to accept it or not, that's their job :)
And I think the most important reason for doing that is because only they can answer the questions. You need to know what the code does to be able to answer things like "is it OK to replace BUG_ON with WARN_ON here?". Although that is preferable according to checkpatch, it's not necessarily a 1-to-1 replacement. The error path changes, and the existing code is likely not tested or developed with the new path in mind.
Nobody(?) in kernelnewbies knows anything about the apparmor code. It's not that we don't want to answer. We just can't.
I think a better approach is to document your thoughts possibly in Documentation/ than posting random patches to teach people what YOU think needs learning. Also people can already, if they need to, learn from patches posted on LKML about what THEY want to learn :). Thanks, Joel
Bjørn
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
participants (4)
-
Bjørn Mork -
Greg KH -
Joel Fernandes -
Walt Feasel