Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-27 Thread Joe Perches 
On Wed, 2015-10-21 at 22:15 -0400, Richard Guy Briggs wrote:
> On 15/10/21, Scott Matheina wrote:
> > On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> > > On 15/10/21, Joe Perches wrote:
> > >> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> > >>> On 15/10/18, Scott Matheina wrote:
> >  On 10/14/2015 04:54 PM, Paul Moore wrote:
> > > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> > >> []
> > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > >> []
> > >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> > >>  {
> > >>  struct audit_entry *e = container_of(head, struct audit_entry, 
> > >> rcu);
> > >>  audit_free_rule(e);
> > >> +
> > >>  }
> > > Why?
> >  I was following the error messages in checkpatch.pl, but the warning
> >  went away after adding this line. No problem with the code. 
> > >>> That sounds like a bug in checkpatch.pl, since that blank line should be
> > >>> tween the declaration and the function call.
> > >> checkpatch message asks for a blank line after the
> > >> "struct audit_entry *e = ..." declaration.
> > > Well then maybe it is a bug in his interpretation of the output of
> > > checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> > > spaces?  Did it pass?
> > 
> > The error did go away. 
> 
> Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
> to me.

It's not a bug in checkpatch.

checkpatch doesn't care if there are blank lines between declarations.

Here's the output of checkpatch for this area:

WARNING: Missing a blank line after declarations
#111: FILE: kernel/auditfilter.c:111:
+   struct audit_entry *e = container_of(head, struct audit_entry, rcu);
+   audit_free_rule(e);

That doesn't suggest putting a blank line before line 111.
It suggests putting a blank line after the declaration of e.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-27 Thread Joe Perches 
On Wed, 2015-10-21 at 22:15 -0400, Richard Guy Briggs wrote:
> On 15/10/21, Scott Matheina wrote:
> > On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> > > On 15/10/21, Joe Perches wrote:
> > >> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> > >>> On 15/10/18, Scott Matheina wrote:
> >  On 10/14/2015 04:54 PM, Paul Moore wrote:
> > > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> > >> []
> > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > >> []
> > >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> > >>  {
> > >>  struct audit_entry *e = container_of(head, struct audit_entry, 
> > >> rcu);
> > >>  audit_free_rule(e);
> > >> +
> > >>  }
> > > Why?
> >  I was following the error messages in checkpatch.pl, but the warning
> >  went away after adding this line. No problem with the code. 
> > >>> That sounds like a bug in checkpatch.pl, since that blank line should be
> > >>> tween the declaration and the function call.
> > >> checkpatch message asks for a blank line after the
> > >> "struct audit_entry *e = ..." declaration.
> > > Well then maybe it is a bug in his interpretation of the output of
> > > checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> > > spaces?  Did it pass?
> > 
> > The error did go away. 
> 
> Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
> to me.

It's not a bug in checkpatch.

checkpatch doesn't care if there are blank lines between declarations.

Here's the output of checkpatch for this area:

WARNING: Missing a blank line after declarations
#111: FILE: kernel/auditfilter.c:111:
+   struct audit_entry *e = container_of(head, struct audit_entry, rcu);
+   audit_free_rule(e);

That doesn't suggest putting a blank line before line 111.
It suggests putting a blank line after the declaration of e.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-25 Thread Scott Matheina 



On 10/21/2015 09:15 PM, Richard Guy Briggs wrote:

On 15/10/21, Scott Matheina wrote:

On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:

On 15/10/21, Joe Perches wrote:

On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:

On 15/10/18, Scott Matheina wrote:

On 10/14/2015 04:54 PM, Paul Moore wrote:

On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:

[]

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c

[]

@@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
  {
struct audit_entry *e = container_of(head, struct audit_entry, rcu);
audit_free_rule(e);
+
  }

Why?

I was following the error messages in checkpatch.pl, but the warning
went away after adding this line. No problem with the code.

That sounds like a bug in checkpatch.pl, since that blank line should be
tween the declaration and the function call.

checkpatch message asks for a blank line after the
"struct audit_entry *e = ..." declaration.

Well then maybe it is a bug in his interpretation of the output of
checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
spaces?  Did it pass?

The error did go away.

Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
to me.  I tried a number of combinations of things and it didn't
complain about several things it should have.  I did try a few other
things to make sure it was still finding problems like brace placement
and leading spaces, but it looks like the blank line checking code isn't
working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
what kernel version are you using?

I had just cloned Linus' repo, so v4.3rc6.



while (*list != ~0U) {
+
unsigned n = *list++;
if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
kfree(p);

Why?

This is the same as above. Just going through the checkpatch.pl
script, and looking for warnings to fix.

Again, another manifestation of that bug?  That blank line should be
after the declaration and before the if statement.

[]

Well, I agree, you have to start somewhere...  Too bad you hit a bug in
checkpatch.pl!

Here too, not a bug in checkpatch.

checkpatch output asks for a blank line after the
"unsigned n" declaration, not before.

- RGB

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-25 Thread Scott Matheina 



On 10/21/2015 09:15 PM, Richard Guy Briggs wrote:

On 15/10/21, Scott Matheina wrote:

On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:

On 15/10/21, Joe Perches wrote:

On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:

On 15/10/18, Scott Matheina wrote:

On 10/14/2015 04:54 PM, Paul Moore wrote:

On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:

[]

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c

[]

@@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
  {
struct audit_entry *e = container_of(head, struct audit_entry, rcu);
audit_free_rule(e);
+
  }

Why?

I was following the error messages in checkpatch.pl, but the warning
went away after adding this line. No problem with the code.

That sounds like a bug in checkpatch.pl, since that blank line should be
tween the declaration and the function call.

checkpatch message asks for a blank line after the
"struct audit_entry *e = ..." declaration.

Well then maybe it is a bug in his interpretation of the output of
checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
spaces?  Did it pass?

The error did go away.

Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
to me.  I tried a number of combinations of things and it didn't
complain about several things it should have.  I did try a few other
things to make sure it was still finding problems like brace placement
and leading spaces, but it looks like the blank line checking code isn't
working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
what kernel version are you using?

I had just cloned Linus' repo, so v4.3rc6.



while (*list != ~0U) {
+
unsigned n = *list++;
if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
kfree(p);

Why?

This is the same as above. Just going through the checkpatch.pl
script, and looking for warnings to fix.

Again, another manifestation of that bug?  That blank line should be
after the declaration and before the if statement.

[]

Well, I agree, you have to start somewhere...  Too bad you hit a bug in
checkpatch.pl!

Here too, not a bug in checkpatch.

checkpatch output asks for a blank line after the
"unsigned n" declaration, not before.

- RGB

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-22 Thread Paul Moore 
On Wed, Oct 21, 2015 at 8:35 PM, Scott Matheina  wrote:
> Thanks for the feedback. I'll resubmit. Now I get to figure out how to 
> resubmit a patch with changes, so a good
> learning experience for me. Pure Hobbyist at this time, but I love to learn.

If you haven't already, go read Documentation/SubmittingPatches in the
kernel source, it will cover a lot of the basics.  Also, as far as I'm
concerned, the #1 absolutely most important thing you must always do
when submitting patches is to make sure that I can simply save your
email and apply it with patch, or import it directly via
stgit/git/whatever; if I have to transform your patch in some way to
get it to apply, I get grumpy, and I don't like to get grumpy.

Beyond that, good luck and have fun :)

-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-22 Thread Richard Guy Briggs 
On 15/10/21, Scott Matheina wrote:
> On 10/21/2015 09:15 PM, Richard Guy Briggs wrote:
> > On 15/10/21, Scott Matheina wrote:
> >> On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> >>> On 15/10/21, Joe Perches wrote:
>  On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> > On 15/10/18, Scott Matheina wrote:
> >> On 10/14/2015 04:54 PM, Paul Moore wrote:
> >>> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
>  []
>  diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>  []
>  @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>   {
>   struct audit_entry *e = container_of(head, struct audit_entry, 
>  rcu);
>   audit_free_rule(e);
>  +
>   }
> >>> Why?
> >> I was following the error messages in checkpatch.pl, but the warning
> >> went away after adding this line. No problem with the code. 
> > That sounds like a bug in checkpatch.pl, since that blank line should be
> > tween the declaration and the function call.
>  checkpatch message asks for a blank line after the
>  "struct audit_entry *e = ..." declaration.
> >>> Well then maybe it is a bug in his interpretation of the output of
> >>> checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> >>> spaces?  Did it pass?
> >> The error did go away. 
> > Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
> > to me.  I tried a number of combinations of things and it didn't
> > complain about several things it should have.  I did try a few other
> > things to make sure it was still finding problems like brace placement
> > and leading spaces, but it looks like the blank line checking code isn't
> > working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
> > what kernel version are you using?
> I'm running Ubuntu 14.04 LTS (Kernel 3.19.0-30-generic) on my machine.
> 
> I cloned Linus' repo for source code. I'm pretty sure you were talking
> about the active Kernel on my machine, so if not please let me know.

I was talking about the source used to generate this patch in question,
run ./scripts/checkpatch.pl and do a compile test.  The active kernel on
your machine is irrelevant unless you subsequently booted it to test it.
How recent is your clone/pull of Linus' repo?

>   while (*list != ~0U) {
>  +
>   unsigned n = *list++;
>   if (n >= AUDIT_BITMASK_SIZE * 32 - 
>  AUDIT_SYSCALL_CLASSES) {
>   kfree(p);
> >>> Why?
> >> This is the same as above. Just going through the checkpatch.pl
> >> script, and looking for warnings to fix. 
> > Again, another manifestation of that bug?  That blank line should be
> > after the declaration and before the if statement.
>  []
> > Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> > checkpatch.pl!
>  Here too, not a bug in checkpatch.
> 
>  checkpatch output asks for a blank line after the
>  "unsigned n" declaration, not before.
> >>> - RGB
> > - RGB

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-22 Thread Paul Moore 
On Wed, Oct 21, 2015 at 8:35 PM, Scott Matheina  wrote:
> Thanks for the feedback. I'll resubmit. Now I get to figure out how to 
> resubmit a patch with changes, so a good
> learning experience for me. Pure Hobbyist at this time, but I love to learn.

If you haven't already, go read Documentation/SubmittingPatches in the
kernel source, it will cover a lot of the basics.  Also, as far as I'm
concerned, the #1 absolutely most important thing you must always do
when submitting patches is to make sure that I can simply save your
email and apply it with patch, or import it directly via
stgit/git/whatever; if I have to transform your patch in some way to
get it to apply, I get grumpy, and I don't like to get grumpy.

Beyond that, good luck and have fun :)

-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-22 Thread Richard Guy Briggs 
On 15/10/21, Scott Matheina wrote:
> On 10/21/2015 09:15 PM, Richard Guy Briggs wrote:
> > On 15/10/21, Scott Matheina wrote:
> >> On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> >>> On 15/10/21, Joe Perches wrote:
>  On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> > On 15/10/18, Scott Matheina wrote:
> >> On 10/14/2015 04:54 PM, Paul Moore wrote:
> >>> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
>  []
>  diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>  []
>  @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>   {
>   struct audit_entry *e = container_of(head, struct audit_entry, 
>  rcu);
>   audit_free_rule(e);
>  +
>   }
> >>> Why?
> >> I was following the error messages in checkpatch.pl, but the warning
> >> went away after adding this line. No problem with the code. 
> > That sounds like a bug in checkpatch.pl, since that blank line should be
> > tween the declaration and the function call.
>  checkpatch message asks for a blank line after the
>  "struct audit_entry *e = ..." declaration.
> >>> Well then maybe it is a bug in his interpretation of the output of
> >>> checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> >>> spaces?  Did it pass?
> >> The error did go away. 
> > Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
> > to me.  I tried a number of combinations of things and it didn't
> > complain about several things it should have.  I did try a few other
> > things to make sure it was still finding problems like brace placement
> > and leading spaces, but it looks like the blank line checking code isn't
> > working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
> > what kernel version are you using?
> I'm running Ubuntu 14.04 LTS (Kernel 3.19.0-30-generic) on my machine.
> 
> I cloned Linus' repo for source code. I'm pretty sure you were talking
> about the active Kernel on my machine, so if not please let me know.

I was talking about the source used to generate this patch in question,
run ./scripts/checkpatch.pl and do a compile test.  The active kernel on
your machine is irrelevant unless you subsequently booted it to test it.
How recent is your clone/pull of Linus' repo?

>   while (*list != ~0U) {
>  +
>   unsigned n = *list++;
>   if (n >= AUDIT_BITMASK_SIZE * 32 - 
>  AUDIT_SYSCALL_CLASSES) {
>   kfree(p);
> >>> Why?
> >> This is the same as above. Just going through the checkpatch.pl
> >> script, and looking for warnings to fix. 
> > Again, another manifestation of that bug?  That blank line should be
> > after the declaration and before the if statement.
>  []
> > Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> > checkpatch.pl!
>  Here too, not a bug in checkpatch.
> 
>  checkpatch output asks for a blank line after the
>  "unsigned n" declaration, not before.
> >>> - RGB
> > - RGB

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Scott Matheina 


On 10/21/2015 09:15 PM, Richard Guy Briggs wrote:
> On 15/10/21, Scott Matheina wrote:
>> On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
>>> On 15/10/21, Joe Perches wrote:
 On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> On 15/10/18, Scott Matheina wrote:
>> On 10/14/2015 04:54 PM, Paul Moore wrote:
>>> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
 []
 diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
 []
 @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
  {
struct audit_entry *e = container_of(head, struct audit_entry, 
 rcu);
audit_free_rule(e);
 +
  }
>>> Why?
>> I was following the error messages in checkpatch.pl, but the warning
>> went away after adding this line. No problem with the code. 
> That sounds like a bug in checkpatch.pl, since that blank line should be
> tween the declaration and the function call.
 checkpatch message asks for a blank line after the
 "struct audit_entry *e = ..." declaration.
>>> Well then maybe it is a bug in his interpretation of the output of
>>> checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
>>> spaces?  Did it pass?
>> The error did go away. 
> Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
> to me.  I tried a number of combinations of things and it didn't
> complain about several things it should have.  I did try a few other
> things to make sure it was still finding problems like brace placement
> and leading spaces, but it looks like the blank line checking code isn't
> working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
> what kernel version are you using?
I'm running Ubuntu 14.04 LTS (Kernel 3.19.0-30-generic) on my machine.

I cloned Linus' repo for source code. I'm pretty sure you were talking about 
the active Kernel on my machine, so if not please let me know.
>
while (*list != ~0U) {
 +
unsigned n = *list++;
if (n >= AUDIT_BITMASK_SIZE * 32 - 
 AUDIT_SYSCALL_CLASSES) {
kfree(p);
>>> Why?
>> This is the same as above. Just going through the checkpatch.pl
>> script, and looking for warnings to fix. 
> Again, another manifestation of that bug?  That blank line should be
> after the declaration and before the if statement.
 []
> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> checkpatch.pl!
 Here too, not a bug in checkpatch.

 checkpatch output asks for a blank line after the
 "unsigned n" declaration, not before.
>>> - RGB
> - RGB
>
> --
> Richard Guy Briggs 
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, 
> Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Richard Guy Briggs 
On 15/10/21, Scott Matheina wrote:
> On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> > On 15/10/21, Joe Perches wrote:
> >> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> >>> On 15/10/18, Scott Matheina wrote:
>  On 10/14/2015 04:54 PM, Paul Moore wrote:
> > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> >> []
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> []
> >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> >>  {
> >>struct audit_entry *e = container_of(head, struct audit_entry, 
> >> rcu);
> >>audit_free_rule(e);
> >> +
> >>  }
> > Why?
>  I was following the error messages in checkpatch.pl, but the warning
>  went away after adding this line. No problem with the code. 
> >>> That sounds like a bug in checkpatch.pl, since that blank line should be
> >>> tween the declaration and the function call.
> >> checkpatch message asks for a blank line after the
> >> "struct audit_entry *e = ..." declaration.
> > Well then maybe it is a bug in his interpretation of the output of
> > checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> > spaces?  Did it pass?
> 
> The error did go away. 

Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
to me.  I tried a number of combinations of things and it didn't
complain about several things it should have.  I did try a few other
things to make sure it was still finding problems like brace placement
and leading spaces, but it looks like the blank line checking code isn't
working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
what kernel version are you using?

> >>while (*list != ~0U) {
> >> +
> >>unsigned n = *list++;
> >>if (n >= AUDIT_BITMASK_SIZE * 32 - 
> >> AUDIT_SYSCALL_CLASSES) {
> >>kfree(p);
> > Why?
>  This is the same as above. Just going through the checkpatch.pl
>  script, and looking for warnings to fix. 
> >>> Again, another manifestation of that bug?  That blank line should be
> >>> after the declaration and before the if statement.
> >> []
> >>> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> >>> checkpatch.pl!
> >> Here too, not a bug in checkpatch.
> >>
> >> checkpatch output asks for a blank line after the
> >> "unsigned n" declaration, not before.
> > - RGB

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Scott Matheina 


On 10/21/2015 01:23 PM, Paul Moore wrote:
> On Sunday, October 18, 2015 12:50:45 PM Scott Matheina wrote:
>> On 10/14/2015 04:54 PM, Paul Moore wrote:
>>> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
 Signed-off-by: Scott Matheina 
 ---

  kernel/auditfilter.c | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)
>>> Sorry for the delay in reviewing this, comments inline ...
>>>
 diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
 index 7714d93..774f9ad 100644
 --- a/kernel/auditfilter.c
 +++ b/kernel/auditfilter.c
 @@ -39,13 +39,13 @@

   * Locking model:
   *

   * audit_filter_mutex:
 - *Synchronizes writes and blocking reads of audit's 
 filterlist
 - *data.  Rcu is used to traverse the filterlist and access
 - *contents of structs audit_entry, audit_watch and opaque
 - *LSM rules during filtering.  If modified, these 
 structures
 - *must be copied and replace their counterparts in the 
 filterlist.
 - *An audit_parent struct is not accessed during 
 filtering, so may
 - *be written directly provided audit_filter_mutex is held.
 + *Synchronizes writes and blocking reads of audit's 
 filterlist
 + *data.  Rcu is used to traverse the filterlist and access
 + *contents of structs audit_entry, audit_watch and opaque
 + *LSM rules during filtering.  If modified, these 
 structures
 + *must be copied and replace their counterparts in the 
 filterlist.
 + *An audit_parent struct is not accessed during 
 filtering, so may
 + *be written directly provided audit_filter_mutex is held.

   */
>>> Okay, that's fine.
> ...
>
>> As you might have guessed, this is one of my first patches. I wasn't sure if
>> a patch like this would even get reviewed, and responded to. I'm subscribed
>> to the linux-kernel mail group, and seeing what is acceptable.
>>
>> Thanks for the review. I don't plan on making a habit of submitting such
>> incredibly trivial patches, but you have to start somewhere, and I thought
>> it'd be hard to screw up by fixing a couple of trivial style errors.
> We all start somewhere, and with that in mind, if you want to resubmit this 
> patch with only the fix above (fixing the whitespace in the comment block), 
> I'll apply it.  While the patch is trivial, it is does fix a minor nit with 
> near-zero risk.
>
> I would encourage you to try something a bit more substantial next time, as 
> they say, bug fixes are the quickest way to a maintainer's heart.
>
Thanks for the feedback. I'll resubmit. Now I get to figure out how to resubmit 
a patch with changes, so a good 
learning experience for me. Pure Hobbyist at this time, but I love to learn. 

Scott 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Scott Matheina 


On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> On 15/10/21, Joe Perches wrote:
>> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
>>> On 15/10/18, Scott Matheina wrote:
 On 10/14/2015 04:54 PM, Paul Moore wrote:
> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
>> []
>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>> []
>> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>>  {
>>  struct audit_entry *e = container_of(head, struct audit_entry, 
>> rcu);
>>  audit_free_rule(e);
>> +
>>  }
> Why?
 I was following the error messages in checkpatch.pl, but the warning
 went away after adding this line. No problem with the code. 
>>> That sounds like a bug in checkpatch.pl, since that blank line should be
>>> tween the declaration and the function call.
>> checkpatch message asks for a blank line after the
>> "struct audit_entry *e = ..." declaration.
> Well then maybe it is a bug in his interpretation of the output of
> checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> spaces?  Did it pass?

The error did go away. 

>>  while (*list != ~0U) {
>> +
>>  unsigned n = *list++;
>>  if (n >= AUDIT_BITMASK_SIZE * 32 - 
>> AUDIT_SYSCALL_CLASSES) {
>>  kfree(p);
> Why?
 This is the same as above. Just going through the checkpatch.pl
 script, and looking for warnings to fix. 
>>> Again, another manifestation of that bug?  That blank line should be
>>> after the declaration and before the if statement.
>> []
>>> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
>>> checkpatch.pl!
>> Here too, not a bug in checkpatch.
>>
>> checkpatch output asks for a blank line after the
>> "unsigned n" declaration, not before.
> - RGB
>
> --
> Richard Guy Briggs 
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, 
> Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Paul Moore 
On Sunday, October 18, 2015 12:50:45 PM Scott Matheina wrote:
> On 10/14/2015 04:54 PM, Paul Moore wrote:
> > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> >> Signed-off-by: Scott Matheina 
> >> ---
> >> 
> >>  kernel/auditfilter.c | 17 ++---
> >>  1 file changed, 10 insertions(+), 7 deletions(-)
> > 
> > Sorry for the delay in reviewing this, comments inline ...
> > 
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> index 7714d93..774f9ad 100644
> >> --- a/kernel/auditfilter.c
> >> +++ b/kernel/auditfilter.c
> >> @@ -39,13 +39,13 @@
> >> 
> >>   * Locking model:
> >>   *
> >> 
> >>   * audit_filter_mutex:
> >> - *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> - *data.  Rcu is used to traverse the filterlist and access
> >> - *contents of structs audit_entry, audit_watch and opaque
> >> - *LSM rules during filtering.  If modified, these 
> >> structures
> >> - *must be copied and replace their counterparts in the 
> >> filterlist.
> >> - *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> - *be written directly provided audit_filter_mutex is held.
> >> + *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> + *data.  Rcu is used to traverse the filterlist and access
> >> + *contents of structs audit_entry, audit_watch and opaque
> >> + *LSM rules during filtering.  If modified, these 
> >> structures
> >> + *must be copied and replace their counterparts in the 
> >> filterlist.
> >> + *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> + *be written directly provided audit_filter_mutex is held.
> >> 
> >>   */
> > 
> > Okay, that's fine.

...

> As you might have guessed, this is one of my first patches. I wasn't sure if
> a patch like this would even get reviewed, and responded to. I'm subscribed
> to the linux-kernel mail group, and seeing what is acceptable.
> 
> Thanks for the review. I don't plan on making a habit of submitting such
> incredibly trivial patches, but you have to start somewhere, and I thought
> it'd be hard to screw up by fixing a couple of trivial style errors.

We all start somewhere, and with that in mind, if you want to resubmit this 
patch with only the fix above (fixing the whitespace in the comment block), 
I'll apply it.  While the patch is trivial, it is does fix a minor nit with 
near-zero risk.

I would encourage you to try something a bit more substantial next time, as 
they say, bug fixes are the quickest way to a maintainer's heart.

-- 
paul moore
www.paul-moore.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Richard Guy Briggs 
On 15/10/21, Joe Perches wrote:
> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> > On 15/10/18, Scott Matheina wrote:
> > > On 10/14/2015 04:54 PM, Paul Moore wrote:
> > > > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> []
> > > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> []
> > > >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> > > >>  {
> > > >>struct audit_entry *e = container_of(head, struct audit_entry, 
> > > >> rcu);
> > > >>audit_free_rule(e);
> > > >> +
> > > >>  }
> > > > Why?
> > > 
> > > I was following the error messages in checkpatch.pl, but the warning
> > > went away after adding this line. No problem with the code. 
> > 
> > That sounds like a bug in checkpatch.pl, since that blank line should be
> > tween the declaration and the function call.
> 
> checkpatch message asks for a blank line after the
> "struct audit_entry *e = ..." declaration.

Well then maybe it is a bug in his interpretation of the output of
checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
spaces?  Did it pass?

> > > >>while (*list != ~0U) {
> > > >> +
> > > >>unsigned n = *list++;
> > > >>if (n >= AUDIT_BITMASK_SIZE * 32 - 
> > > >> AUDIT_SYSCALL_CLASSES) {
> > > >>kfree(p);
> > > > Why?
> > > 
> > > This is the same as above. Just going through the checkpatch.pl
> > > script, and looking for warnings to fix. 
> > 
> > Again, another manifestation of that bug?  That blank line should be
> > after the declaration and before the if statement.
> []
> > Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> > checkpatch.pl!
> 
> Here too, not a bug in checkpatch.
> 
> checkpatch output asks for a blank line after the
> "unsigned n" declaration, not before.

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Joe Perches 
On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> On 15/10/18, Scott Matheina wrote:
> > On 10/14/2015 04:54 PM, Paul Moore wrote:
> > > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
[]
> > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
[]
> > >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> > >>  {
> > >>  struct audit_entry *e = container_of(head, struct audit_entry, 
> > >> rcu);
> > >>  audit_free_rule(e);
> > >> +
> > >>  }
> > > Why?
> > 
> > I was following the error messages in checkpatch.pl, but the warning
> > went away after adding this line. No problem with the code. 
> 
> That sounds like a bug in checkpatch.pl, since that blank line should be
> tween the declaration and the function call.

checkpatch message asks for a blank line after the
"struct audit_entry *e = ..." declaration.

> > >>  while (*list != ~0U) {
> > >> +
> > >>  unsigned n = *list++;
> > >>  if (n >= AUDIT_BITMASK_SIZE * 32 - 
> > >> AUDIT_SYSCALL_CLASSES) {
> > >>  kfree(p);
> > > Why?
> > 
> > This is the same as above. Just going through the checkpatch.pl
> > script, and looking for warnings to fix. 
> 
> Again, another manifestation of that bug?  That blank line should be
> after the declaration and before the if statement.
[]
> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> checkpatch.pl!

Here too, not a bug in checkpatch.

checkpatch output asks for a blank line after the
"unsigned n" declaration, not before.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Joe Perches 
On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> On 15/10/18, Scott Matheina wrote:
> > On 10/14/2015 04:54 PM, Paul Moore wrote:
> > > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
[]
> > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
[]
> > >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> > >>  {
> > >>  struct audit_entry *e = container_of(head, struct audit_entry, 
> > >> rcu);
> > >>  audit_free_rule(e);
> > >> +
> > >>  }
> > > Why?
> > 
> > I was following the error messages in checkpatch.pl, but the warning
> > went away after adding this line. No problem with the code. 
> 
> That sounds like a bug in checkpatch.pl, since that blank line should be
> tween the declaration and the function call.

checkpatch message asks for a blank line after the
"struct audit_entry *e = ..." declaration.

> > >>  while (*list != ~0U) {
> > >> +
> > >>  unsigned n = *list++;
> > >>  if (n >= AUDIT_BITMASK_SIZE * 32 - 
> > >> AUDIT_SYSCALL_CLASSES) {
> > >>  kfree(p);
> > > Why?
> > 
> > This is the same as above. Just going through the checkpatch.pl
> > script, and looking for warnings to fix. 
> 
> Again, another manifestation of that bug?  That blank line should be
> after the declaration and before the if statement.
[]
> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> checkpatch.pl!

Here too, not a bug in checkpatch.

checkpatch output asks for a blank line after the
"unsigned n" declaration, not before.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Richard Guy Briggs 
On 15/10/21, Joe Perches wrote:
> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> > On 15/10/18, Scott Matheina wrote:
> > > On 10/14/2015 04:54 PM, Paul Moore wrote:
> > > > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> []
> > > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> []
> > > >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> > > >>  {
> > > >>struct audit_entry *e = container_of(head, struct audit_entry, 
> > > >> rcu);
> > > >>audit_free_rule(e);
> > > >> +
> > > >>  }
> > > > Why?
> > > 
> > > I was following the error messages in checkpatch.pl, but the warning
> > > went away after adding this line. No problem with the code. 
> > 
> > That sounds like a bug in checkpatch.pl, since that blank line should be
> > tween the declaration and the function call.
> 
> checkpatch message asks for a blank line after the
> "struct audit_entry *e = ..." declaration.

Well then maybe it is a bug in his interpretation of the output of
checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
spaces?  Did it pass?

> > > >>while (*list != ~0U) {
> > > >> +
> > > >>unsigned n = *list++;
> > > >>if (n >= AUDIT_BITMASK_SIZE * 32 - 
> > > >> AUDIT_SYSCALL_CLASSES) {
> > > >>kfree(p);
> > > > Why?
> > > 
> > > This is the same as above. Just going through the checkpatch.pl
> > > script, and looking for warnings to fix. 
> > 
> > Again, another manifestation of that bug?  That blank line should be
> > after the declaration and before the if statement.
> []
> > Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> > checkpatch.pl!
> 
> Here too, not a bug in checkpatch.
> 
> checkpatch output asks for a blank line after the
> "unsigned n" declaration, not before.

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Paul Moore 
On Sunday, October 18, 2015 12:50:45 PM Scott Matheina wrote:
> On 10/14/2015 04:54 PM, Paul Moore wrote:
> > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> >> Signed-off-by: Scott Matheina 
> >> ---
> >> 
> >>  kernel/auditfilter.c | 17 ++---
> >>  1 file changed, 10 insertions(+), 7 deletions(-)
> > 
> > Sorry for the delay in reviewing this, comments inline ...
> > 
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> index 7714d93..774f9ad 100644
> >> --- a/kernel/auditfilter.c
> >> +++ b/kernel/auditfilter.c
> >> @@ -39,13 +39,13 @@
> >> 
> >>   * Locking model:
> >>   *
> >> 
> >>   * audit_filter_mutex:
> >> - *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> - *data.  Rcu is used to traverse the filterlist and access
> >> - *contents of structs audit_entry, audit_watch and opaque
> >> - *LSM rules during filtering.  If modified, these 
> >> structures
> >> - *must be copied and replace their counterparts in the 
> >> filterlist.
> >> - *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> - *be written directly provided audit_filter_mutex is held.
> >> + *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> + *data.  Rcu is used to traverse the filterlist and access
> >> + *contents of structs audit_entry, audit_watch and opaque
> >> + *LSM rules during filtering.  If modified, these 
> >> structures
> >> + *must be copied and replace their counterparts in the 
> >> filterlist.
> >> + *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> + *be written directly provided audit_filter_mutex is held.
> >> 
> >>   */
> > 
> > Okay, that's fine.

...

> As you might have guessed, this is one of my first patches. I wasn't sure if
> a patch like this would even get reviewed, and responded to. I'm subscribed
> to the linux-kernel mail group, and seeing what is acceptable.
> 
> Thanks for the review. I don't plan on making a habit of submitting such
> incredibly trivial patches, but you have to start somewhere, and I thought
> it'd be hard to screw up by fixing a couple of trivial style errors.

We all start somewhere, and with that in mind, if you want to resubmit this 
patch with only the fix above (fixing the whitespace in the comment block), 
I'll apply it.  While the patch is trivial, it is does fix a minor nit with 
near-zero risk.

I would encourage you to try something a bit more substantial next time, as 
they say, bug fixes are the quickest way to a maintainer's heart.

-- 
paul moore
www.paul-moore.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Richard Guy Briggs 
On 15/10/21, Scott Matheina wrote:
> On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> > On 15/10/21, Joe Perches wrote:
> >> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> >>> On 15/10/18, Scott Matheina wrote:
>  On 10/14/2015 04:54 PM, Paul Moore wrote:
> > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> >> []
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> []
> >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> >>  {
> >>struct audit_entry *e = container_of(head, struct audit_entry, 
> >> rcu);
> >>audit_free_rule(e);
> >> +
> >>  }
> > Why?
>  I was following the error messages in checkpatch.pl, but the warning
>  went away after adding this line. No problem with the code. 
> >>> That sounds like a bug in checkpatch.pl, since that blank line should be
> >>> tween the declaration and the function call.
> >> checkpatch message asks for a blank line after the
> >> "struct audit_entry *e = ..." declaration.
> > Well then maybe it is a bug in his interpretation of the output of
> > checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> > spaces?  Did it pass?
> 
> The error did go away. 

Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
to me.  I tried a number of combinations of things and it didn't
complain about several things it should have.  I did try a few other
things to make sure it was still finding problems like brace placement
and leading spaces, but it looks like the blank line checking code isn't
working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
what kernel version are you using?

> >>while (*list != ~0U) {
> >> +
> >>unsigned n = *list++;
> >>if (n >= AUDIT_BITMASK_SIZE * 32 - 
> >> AUDIT_SYSCALL_CLASSES) {
> >>kfree(p);
> > Why?
>  This is the same as above. Just going through the checkpatch.pl
>  script, and looking for warnings to fix. 
> >>> Again, another manifestation of that bug?  That blank line should be
> >>> after the declaration and before the if statement.
> >> []
> >>> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> >>> checkpatch.pl!
> >> Here too, not a bug in checkpatch.
> >>
> >> checkpatch output asks for a blank line after the
> >> "unsigned n" declaration, not before.
> > - RGB

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Scott Matheina 


On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
> On 15/10/21, Joe Perches wrote:
>> On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
>>> On 15/10/18, Scott Matheina wrote:
 On 10/14/2015 04:54 PM, Paul Moore wrote:
> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
>> []
>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>> []
>> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>>  {
>>  struct audit_entry *e = container_of(head, struct audit_entry, 
>> rcu);
>>  audit_free_rule(e);
>> +
>>  }
> Why?
 I was following the error messages in checkpatch.pl, but the warning
 went away after adding this line. No problem with the code. 
>>> That sounds like a bug in checkpatch.pl, since that blank line should be
>>> tween the declaration and the function call.
>> checkpatch message asks for a blank line after the
>> "struct audit_entry *e = ..." declaration.
> Well then maybe it is a bug in his interpretation of the output of
> checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
> spaces?  Did it pass?

The error did go away. 

>>  while (*list != ~0U) {
>> +
>>  unsigned n = *list++;
>>  if (n >= AUDIT_BITMASK_SIZE * 32 - 
>> AUDIT_SYSCALL_CLASSES) {
>>  kfree(p);
> Why?
 This is the same as above. Just going through the checkpatch.pl
 script, and looking for warnings to fix. 
>>> Again, another manifestation of that bug?  That blank line should be
>>> after the declaration and before the if statement.
>> []
>>> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
>>> checkpatch.pl!
>> Here too, not a bug in checkpatch.
>>
>> checkpatch output asks for a blank line after the
>> "unsigned n" declaration, not before.
> - RGB
>
> --
> Richard Guy Briggs 
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, 
> Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Scott Matheina 


On 10/21/2015 01:23 PM, Paul Moore wrote:
> On Sunday, October 18, 2015 12:50:45 PM Scott Matheina wrote:
>> On 10/14/2015 04:54 PM, Paul Moore wrote:
>>> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
 Signed-off-by: Scott Matheina 
 ---

  kernel/auditfilter.c | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)
>>> Sorry for the delay in reviewing this, comments inline ...
>>>
 diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
 index 7714d93..774f9ad 100644
 --- a/kernel/auditfilter.c
 +++ b/kernel/auditfilter.c
 @@ -39,13 +39,13 @@

   * Locking model:
   *

   * audit_filter_mutex:
 - *Synchronizes writes and blocking reads of audit's 
 filterlist
 - *data.  Rcu is used to traverse the filterlist and access
 - *contents of structs audit_entry, audit_watch and opaque
 - *LSM rules during filtering.  If modified, these 
 structures
 - *must be copied and replace their counterparts in the 
 filterlist.
 - *An audit_parent struct is not accessed during 
 filtering, so may
 - *be written directly provided audit_filter_mutex is held.
 + *Synchronizes writes and blocking reads of audit's 
 filterlist
 + *data.  Rcu is used to traverse the filterlist and access
 + *contents of structs audit_entry, audit_watch and opaque
 + *LSM rules during filtering.  If modified, these 
 structures
 + *must be copied and replace their counterparts in the 
 filterlist.
 + *An audit_parent struct is not accessed during 
 filtering, so may
 + *be written directly provided audit_filter_mutex is held.

   */
>>> Okay, that's fine.
> ...
>
>> As you might have guessed, this is one of my first patches. I wasn't sure if
>> a patch like this would even get reviewed, and responded to. I'm subscribed
>> to the linux-kernel mail group, and seeing what is acceptable.
>>
>> Thanks for the review. I don't plan on making a habit of submitting such
>> incredibly trivial patches, but you have to start somewhere, and I thought
>> it'd be hard to screw up by fixing a couple of trivial style errors.
> We all start somewhere, and with that in mind, if you want to resubmit this 
> patch with only the fix above (fixing the whitespace in the comment block), 
> I'll apply it.  While the patch is trivial, it is does fix a minor nit with 
> near-zero risk.
>
> I would encourage you to try something a bit more substantial next time, as 
> they say, bug fixes are the quickest way to a maintainer's heart.
>
Thanks for the feedback. I'll resubmit. Now I get to figure out how to resubmit 
a patch with changes, so a good 
learning experience for me. Pure Hobbyist at this time, but I love to learn. 

Scott 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-21 Thread Scott Matheina 


On 10/21/2015 09:15 PM, Richard Guy Briggs wrote:
> On 15/10/21, Scott Matheina wrote:
>> On 10/21/2015 10:33 AM, Richard Guy Briggs wrote:
>>> On 15/10/21, Joe Perches wrote:
 On Mon, 2015-10-19 at 12:10 -0400, Richard Guy Briggs wrote:
> On 15/10/18, Scott Matheina wrote:
>> On 10/14/2015 04:54 PM, Paul Moore wrote:
>>> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
 []
 diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
 []
 @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
  {
struct audit_entry *e = container_of(head, struct audit_entry, 
 rcu);
audit_free_rule(e);
 +
  }
>>> Why?
>> I was following the error messages in checkpatch.pl, but the warning
>> went away after adding this line. No problem with the code. 
> That sounds like a bug in checkpatch.pl, since that blank line should be
> tween the declaration and the function call.
 checkpatch message asks for a blank line after the
 "struct audit_entry *e = ..." declaration.
>>> Well then maybe it is a bug in his interpretation of the output of
>>> checkpatch.pl?  Scott, did you re-run checkpatch.pl after adding those
>>> spaces?  Did it pass?
>> The error did go away. 
> Joe, I confirm the error went away.  Looks like a bug in checkpatch.pl
> to me.  I tried a number of combinations of things and it didn't
> complain about several things it should have.  I did try a few other
> things to make sure it was still finding problems like brace placement
> and leading spaces, but it looks like the blank line checking code isn't
> working.  This is on 4.0, so maybe it has been fixed since then.  Scott,
> what kernel version are you using?
I'm running Ubuntu 14.04 LTS (Kernel 3.19.0-30-generic) on my machine.

I cloned Linus' repo for source code. I'm pretty sure you were talking about 
the active Kernel on my machine, so if not please let me know.
>
while (*list != ~0U) {
 +
unsigned n = *list++;
if (n >= AUDIT_BITMASK_SIZE * 32 - 
 AUDIT_SYSCALL_CLASSES) {
kfree(p);
>>> Why?
>> This is the same as above. Just going through the checkpatch.pl
>> script, and looking for warnings to fix. 
> Again, another manifestation of that bug?  That blank line should be
> after the declaration and before the if statement.
 []
> Well, I agree, you have to start somewhere...  Too bad you hit a bug in
> checkpatch.pl!
 Here too, not a bug in checkpatch.

 checkpatch output asks for a blank line after the
 "unsigned n" declaration, not before.
>>> - RGB
> - RGB
>
> --
> Richard Guy Briggs 
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, 
> Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-19 Thread Richard Guy Briggs 
On 15/10/18, Scott Matheina wrote:
> On 10/14/2015 04:54 PM, Paul Moore wrote:
> > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> >> Signed-off-by: Scott Matheina 
> >> ---
> >>  kernel/auditfilter.c | 17 ++---
> >>  1 file changed, 10 insertions(+), 7 deletions(-)
> > Sorry for the delay in reviewing this, comments inline ...
> >
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> index 7714d93..774f9ad 100644
> >> --- a/kernel/auditfilter.c
> >> +++ b/kernel/auditfilter.c
> >> @@ -39,13 +39,13 @@
> >>   * Locking model:
> >>   *
> >>   * audit_filter_mutex:
> >> - *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> - *data.  Rcu is used to traverse the filterlist and access
> >> - *contents of structs audit_entry, audit_watch and opaque
> >> - *LSM rules during filtering.  If modified, these 
> >> structures
> >> - *must be copied and replace their counterparts in the 
> >> filterlist.
> >> - *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> - *be written directly provided audit_filter_mutex is held.
> >> + *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> + *data.  Rcu is used to traverse the filterlist and access
> >> + *contents of structs audit_entry, audit_watch and opaque
> >> + *LSM rules during filtering.  If modified, these 
> >> structures
> >> + *must be copied and replace their counterparts in the 
> >> filterlist.
> >> + *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> + *be written directly provided audit_filter_mutex is held.
> >>   */
> > Okay, that's fine.
> >
> >>  /* Audit filter lists, defined in  */
> >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> >>  {
> >>struct audit_entry *e = container_of(head, struct audit_entry, rcu);
> >>audit_free_rule(e);
> >> +
> >>  }
> > Why?
> 
> I was following the error messages in checkpatch.pl, but the warning
> went away after adding this line. No problem with the code. 

That sounds like a bug in checkpatch.pl, since that blank line should be
tween the declaration and the function call.

> >>  /* Initialize an audit filterlist entry. */
> >> @@ -176,9 +177,11 @@ static __u32 *classes[AUDIT_SYSCALL_CLASSES];
> >>  int __init audit_register_class(int class, unsigned *list)
> >>  {
> >>__u32 *p = kcalloc(AUDIT_BITMASK_SIZE, sizeof(__u32), GFP_KERNEL);
> >> +
> >>if (!p)
> >>return -ENOMEM;
> > Okay.
> >
> >>while (*list != ~0U) {
> >> +
> >>unsigned n = *list++;
> >>if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
> >>kfree(p);
> > Why?
> 
> This is the same as above. Just going through the checkpatch.pl
> script, and looking for warnings to fix. 

Again, another manifestation of that bug?  That blank line should be
after the declaration and before the if statement.

> As you might have guessed, this is one of my first patches. I wasn't
> sure if a patch like this would even get reviewed, and responded to.
> I'm subscribed to the linux-kernel mail group, and seeing what is
> acceptable. 
> 
> Thanks for the review. I don't plan on making a habit of submitting
> such incredibly trivial patches, but you have to start somewhere, and
> I thought it'd be hard to screw up by fixing a couple of trivial style
> errors.  

Well, I agree, you have to start somewhere...  Too bad you hit a bug in
checkpatch.pl!

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-19 Thread Richard Guy Briggs 
On 15/10/18, Scott Matheina wrote:
> On 10/14/2015 04:54 PM, Paul Moore wrote:
> > On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> >> Signed-off-by: Scott Matheina 
> >> ---
> >>  kernel/auditfilter.c | 17 ++---
> >>  1 file changed, 10 insertions(+), 7 deletions(-)
> > Sorry for the delay in reviewing this, comments inline ...
> >
> >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> >> index 7714d93..774f9ad 100644
> >> --- a/kernel/auditfilter.c
> >> +++ b/kernel/auditfilter.c
> >> @@ -39,13 +39,13 @@
> >>   * Locking model:
> >>   *
> >>   * audit_filter_mutex:
> >> - *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> - *data.  Rcu is used to traverse the filterlist and access
> >> - *contents of structs audit_entry, audit_watch and opaque
> >> - *LSM rules during filtering.  If modified, these 
> >> structures
> >> - *must be copied and replace their counterparts in the 
> >> filterlist.
> >> - *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> - *be written directly provided audit_filter_mutex is held.
> >> + *Synchronizes writes and blocking reads of audit's 
> >> filterlist
> >> + *data.  Rcu is used to traverse the filterlist and access
> >> + *contents of structs audit_entry, audit_watch and opaque
> >> + *LSM rules during filtering.  If modified, these 
> >> structures
> >> + *must be copied and replace their counterparts in the 
> >> filterlist.
> >> + *An audit_parent struct is not accessed during 
> >> filtering, so may
> >> + *be written directly provided audit_filter_mutex is held.
> >>   */
> > Okay, that's fine.
> >
> >>  /* Audit filter lists, defined in  */
> >> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
> >>  {
> >>struct audit_entry *e = container_of(head, struct audit_entry, rcu);
> >>audit_free_rule(e);
> >> +
> >>  }
> > Why?
> 
> I was following the error messages in checkpatch.pl, but the warning
> went away after adding this line. No problem with the code. 

That sounds like a bug in checkpatch.pl, since that blank line should be
tween the declaration and the function call.

> >>  /* Initialize an audit filterlist entry. */
> >> @@ -176,9 +177,11 @@ static __u32 *classes[AUDIT_SYSCALL_CLASSES];
> >>  int __init audit_register_class(int class, unsigned *list)
> >>  {
> >>__u32 *p = kcalloc(AUDIT_BITMASK_SIZE, sizeof(__u32), GFP_KERNEL);
> >> +
> >>if (!p)
> >>return -ENOMEM;
> > Okay.
> >
> >>while (*list != ~0U) {
> >> +
> >>unsigned n = *list++;
> >>if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
> >>kfree(p);
> > Why?
> 
> This is the same as above. Just going through the checkpatch.pl
> script, and looking for warnings to fix. 

Again, another manifestation of that bug?  That blank line should be
after the declaration and before the if statement.

> As you might have guessed, this is one of my first patches. I wasn't
> sure if a patch like this would even get reviewed, and responded to.
> I'm subscribed to the linux-kernel mail group, and seeing what is
> acceptable. 
> 
> Thanks for the review. I don't plan on making a habit of submitting
> such incredibly trivial patches, but you have to start somewhere, and
> I thought it'd be hard to screw up by fixing a couple of trivial style
> errors.  

Well, I agree, you have to start somewhere...  Too bad you hit a bug in
checkpatch.pl!

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-18 Thread Scott Matheina 


On 10/14/2015 04:54 PM, Paul Moore wrote:
> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
>> Signed-off-by: Scott Matheina 
>> ---
>>  kernel/auditfilter.c | 17 ++---
>>  1 file changed, 10 insertions(+), 7 deletions(-)
> Sorry for the delay in reviewing this, comments inline ...
>
>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>> index 7714d93..774f9ad 100644
>> --- a/kernel/auditfilter.c
>> +++ b/kernel/auditfilter.c
>> @@ -39,13 +39,13 @@
>>   * Locking model:
>>   *
>>   * audit_filter_mutex:
>> - *  Synchronizes writes and blocking reads of audit's filterlist
>> - *  data.  Rcu is used to traverse the filterlist and access
>> - *  contents of structs audit_entry, audit_watch and opaque
>> - *  LSM rules during filtering.  If modified, these structures
>> - *  must be copied and replace their counterparts in the filterlist.
>> - *  An audit_parent struct is not accessed during filtering, so may
>> - *  be written directly provided audit_filter_mutex is held.
>> + *  Synchronizes writes and blocking reads of audit's filterlist
>> + *  data.  Rcu is used to traverse the filterlist and access
>> + *  contents of structs audit_entry, audit_watch and opaque
>> + *  LSM rules during filtering.  If modified, these structures
>> + *  must be copied and replace their counterparts in the filterlist.
>> + *  An audit_parent struct is not accessed during filtering, so may
>> + *  be written directly provided audit_filter_mutex is held.
>>   */
> Okay, that's fine.
>
>>  /* Audit filter lists, defined in  */
>> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>>  {
>>  struct audit_entry *e = container_of(head, struct audit_entry, rcu);
>>  audit_free_rule(e);
>> +
>>  }
> Why?

I was following the error messages in checkpatch.pl, but the warning went away 
after adding this line. No problem
with the code. 

>>  /* Initialize an audit filterlist entry. */
>> @@ -176,9 +177,11 @@ static __u32 *classes[AUDIT_SYSCALL_CLASSES];
>>  int __init audit_register_class(int class, unsigned *list)
>>  {
>>  __u32 *p = kcalloc(AUDIT_BITMASK_SIZE, sizeof(__u32), GFP_KERNEL);
>> +
>>  if (!p)
>>  return -ENOMEM;
> Okay.
>
>>  while (*list != ~0U) {
>> +
>>  unsigned n = *list++;
>>  if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
>>  kfree(p);
> Why?

This is the same as above. Just going through the checkpatch.pl script, and 
looking for warnings to fix. 

As you might have guessed, this is one of my first patches. I wasn't sure if a 
patch like this would even get
reviewed, and responded to. I'm subscribed to the linux-kernel mail group, and 
seeing what is acceptable. 

Thanks for the review. I don't plan on making a habit of submitting such 
incredibly trivial patches, but you 
have to start somewhere, and I thought it'd be hard to screw up by fixing a 
couple of trivial style errors.  


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-18 Thread Scott Matheina 


On 10/14/2015 04:54 PM, Paul Moore wrote:
> On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
>> Signed-off-by: Scott Matheina 
>> ---
>>  kernel/auditfilter.c | 17 ++---
>>  1 file changed, 10 insertions(+), 7 deletions(-)
> Sorry for the delay in reviewing this, comments inline ...
>
>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>> index 7714d93..774f9ad 100644
>> --- a/kernel/auditfilter.c
>> +++ b/kernel/auditfilter.c
>> @@ -39,13 +39,13 @@
>>   * Locking model:
>>   *
>>   * audit_filter_mutex:
>> - *  Synchronizes writes and blocking reads of audit's filterlist
>> - *  data.  Rcu is used to traverse the filterlist and access
>> - *  contents of structs audit_entry, audit_watch and opaque
>> - *  LSM rules during filtering.  If modified, these structures
>> - *  must be copied and replace their counterparts in the filterlist.
>> - *  An audit_parent struct is not accessed during filtering, so may
>> - *  be written directly provided audit_filter_mutex is held.
>> + *  Synchronizes writes and blocking reads of audit's filterlist
>> + *  data.  Rcu is used to traverse the filterlist and access
>> + *  contents of structs audit_entry, audit_watch and opaque
>> + *  LSM rules during filtering.  If modified, these structures
>> + *  must be copied and replace their counterparts in the filterlist.
>> + *  An audit_parent struct is not accessed during filtering, so may
>> + *  be written directly provided audit_filter_mutex is held.
>>   */
> Okay, that's fine.
>
>>  /* Audit filter lists, defined in  */
>> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>>  {
>>  struct audit_entry *e = container_of(head, struct audit_entry, rcu);
>>  audit_free_rule(e);
>> +
>>  }
> Why?

I was following the error messages in checkpatch.pl, but the warning went away 
after adding this line. No problem
with the code. 

>>  /* Initialize an audit filterlist entry. */
>> @@ -176,9 +177,11 @@ static __u32 *classes[AUDIT_SYSCALL_CLASSES];
>>  int __init audit_register_class(int class, unsigned *list)
>>  {
>>  __u32 *p = kcalloc(AUDIT_BITMASK_SIZE, sizeof(__u32), GFP_KERNEL);
>> +
>>  if (!p)
>>  return -ENOMEM;
> Okay.
>
>>  while (*list != ~0U) {
>> +
>>  unsigned n = *list++;
>>  if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
>>  kfree(p);
> Why?

This is the same as above. Just going through the checkpatch.pl script, and 
looking for warnings to fix. 

As you might have guessed, this is one of my first patches. I wasn't sure if a 
patch like this would even get
reviewed, and responded to. I'm subscribed to the linux-kernel mail group, and 
seeing what is acceptable. 

Thanks for the review. I don't plan on making a habit of submitting such 
incredibly trivial patches, but you 
have to start somewhere, and I thought it'd be hard to screw up by fixing a 
couple of trivial style errors.  


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-14 Thread Paul Moore 
On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> Signed-off-by: Scott Matheina 
> ---
>  kernel/auditfilter.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)

Sorry for the delay in reviewing this, comments inline ...

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 7714d93..774f9ad 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -39,13 +39,13 @@
>   * Locking model:
>   *
>   * audit_filter_mutex:
> - *   Synchronizes writes and blocking reads of audit's filterlist
> - *   data.  Rcu is used to traverse the filterlist and access
> - *   contents of structs audit_entry, audit_watch and opaque
> - *   LSM rules during filtering.  If modified, these structures
> - *   must be copied and replace their counterparts in the filterlist.
> - *   An audit_parent struct is not accessed during filtering, so may
> - *   be written directly provided audit_filter_mutex is held.
> + *   Synchronizes writes and blocking reads of audit's filterlist
> + *   data.  Rcu is used to traverse the filterlist and access
> + *   contents of structs audit_entry, audit_watch and opaque
> + *   LSM rules during filtering.  If modified, these structures
> + *   must be copied and replace their counterparts in the filterlist.
> + *   An audit_parent struct is not accessed during filtering, so may
> + *   be written directly provided audit_filter_mutex is held.
>   */

Okay, that's fine.

>  /* Audit filter lists, defined in  */
> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>  {
>   struct audit_entry *e = container_of(head, struct audit_entry, rcu);
>   audit_free_rule(e);
> +
>  }

Why?

>  /* Initialize an audit filterlist entry. */
> @@ -176,9 +177,11 @@ static __u32 *classes[AUDIT_SYSCALL_CLASSES];
>  int __init audit_register_class(int class, unsigned *list)
>  {
>   __u32 *p = kcalloc(AUDIT_BITMASK_SIZE, sizeof(__u32), GFP_KERNEL);
> +
>   if (!p)
>   return -ENOMEM;

Okay.

>   while (*list != ~0U) {
> +
>   unsigned n = *list++;
>   if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
>   kfree(p);

Why?

-- 
paul moore
www.paul-moore.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] Fixed Trivial Warnings in file: Deleted Spaces prior to tabs, and added lines. modified: kernel/auditfilter.c

2015-10-14 Thread Paul Moore 
On Saturday, October 10, 2015 08:57:55 PM Scott Matheina wrote:
> Signed-off-by: Scott Matheina 
> ---
>  kernel/auditfilter.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)

Sorry for the delay in reviewing this, comments inline ...

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 7714d93..774f9ad 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -39,13 +39,13 @@
>   * Locking model:
>   *
>   * audit_filter_mutex:
> - *   Synchronizes writes and blocking reads of audit's filterlist
> - *   data.  Rcu is used to traverse the filterlist and access
> - *   contents of structs audit_entry, audit_watch and opaque
> - *   LSM rules during filtering.  If modified, these structures
> - *   must be copied and replace their counterparts in the filterlist.
> - *   An audit_parent struct is not accessed during filtering, so may
> - *   be written directly provided audit_filter_mutex is held.
> + *   Synchronizes writes and blocking reads of audit's filterlist
> + *   data.  Rcu is used to traverse the filterlist and access
> + *   contents of structs audit_entry, audit_watch and opaque
> + *   LSM rules during filtering.  If modified, these structures
> + *   must be copied and replace their counterparts in the filterlist.
> + *   An audit_parent struct is not accessed during filtering, so may
> + *   be written directly provided audit_filter_mutex is held.
>   */

Okay, that's fine.

>  /* Audit filter lists, defined in  */
> @@ -109,6 +109,7 @@ void audit_free_rule_rcu(struct rcu_head *head)
>  {
>   struct audit_entry *e = container_of(head, struct audit_entry, rcu);
>   audit_free_rule(e);
> +
>  }

Why?

>  /* Initialize an audit filterlist entry. */
> @@ -176,9 +177,11 @@ static __u32 *classes[AUDIT_SYSCALL_CLASSES];
>  int __init audit_register_class(int class, unsigned *list)
>  {
>   __u32 *p = kcalloc(AUDIT_BITMASK_SIZE, sizeof(__u32), GFP_KERNEL);
> +
>   if (!p)
>   return -ENOMEM;

Okay.

>   while (*list != ~0U) {
> +
>   unsigned n = *list++;
>   if (n >= AUDIT_BITMASK_SIZE * 32 - AUDIT_SYSCALL_CLASSES) {
>   kfree(p);

Why?

-- 
paul moore
www.paul-moore.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/