security: apparmor: apparmorfs.c Checkpatch mods
Subsystems (2)
cover security: apparmor: apparmorfs.c Checkpatch mods
Make checkpatch modifications to include:
Multiple blank lines
Blank line after declarations
Space after castWARN_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
PATCH 1/7 security: apparmor: apparmorfs.c Multiple blank lines
Make style modifications for:
CHECK: Please don't use multiple blank lines
--- security/apparmor/apparmorfs.c | 4 ---- 1 file changed, 4 deletions(-)
security/apparmor/apparmorfs.c +0 -4
@@ -112,7 +112,6 @@ static char *aa_simple_write_to_buffer(int op, const char __user *userbuf,return data; } -/* .load file hook fn to load policy */ static ssize_t profile_load (struct file *f,const char __user *buf,size_t size,loff_t *pos)
@@ -552,7 +551,6 @@ int __aa_fs_namespace_mkdir(struct aa_namespace *ns, struct dentry *parent,return error; } -#define list_entry_is_head (pos, head, member) (&pos->member == (head))/**
@@ -687,7 +685,6 @@ static void *p_start(struct seq_file *f, loff_t *pos)loff_t l = *pos; f->private =aa_get_namespace (root); -/* find the first profile */ mutex_lock (&root->lock); profile =__first_profile (root, root);
@@ -782,7 +779,6 @@ static const struct file_operations aa_fs_profiles_fops = { .release = profiles_release, }; -/** Base file system setup **/ static struct aa_fs_entry aa_fs_entry_file[] = { AA_FS_FILE_STRING("mask", "create read write exec append mmap_exec " \
Signature · 1 line
2.1.4
PATCH 2/7 security: apparmor: apparmorfs.c Blank line after declarations
In reply to Walt FeaselMake style modifications for:
WARNING: Missing a blank line after declarations
--- security/apparmor/apparmorfs.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
security/apparmor/apparmorfs.c +10 -0
@@ -57,6 +57,7 @@ static int mangle_name(char *name, char *target) *t =0 ; }else {int len =0 ; +for (; *name; name++) {if (isalnum (*name) ||isspace (*name) ||strchr ("/._-" , *name))
@@ -240,6 +241,7 @@ static int aa_fs_seq_profile_open(struct inode *inode, struct file *file,static int aa_fs_seq_profile_release (struct inode *inode,struct file *file) {struct seq_file *seq = (struct seq_file *) file->private_data; +if (seq)aa_put_replacedby (seq->private);return single_release (inode, file);
@@ -249,6 +251,7 @@ static int aa_fs_seq_profname_show(struct seq_file *seq, void *v) {struct aa_replacedby *r = seq->private;struct aa_profile *profile =aa_get_profile_rcu (&r->profile); +seq_printf (seq,"%s\n" , profile->base.name);aa_put_profile (profile);
@@ -272,6 +275,7 @@ static int aa_fs_seq_profmode_show(struct seq_file *seq, void *v) {struct aa_replacedby *r = seq->private;struct aa_profile *profile =aa_get_profile_rcu (&r->profile); +seq_printf (seq,"%s\n" , aa_profile_mode_names[profile->mode]);aa_put_profile (profile);
@@ -295,6 +299,7 @@ static int aa_fs_seq_profattach_show(struct seq_file *seq, void *v) {struct aa_replacedby *r = seq->private;struct aa_profile *profile =aa_get_profile_rcu (&r->profile); +if (profile->attach)seq_printf (seq,"%s\n" , profile->attach);else if (profile->xmatch)
@@ -362,6 +367,7 @@ void __aa_fs_profile_rmdir(struct aa_profile *profile)for (i =AAFS_PROF_SIZEOF -1 ; i >=0 ; --i) {struct aa_replacedby *r; +if (!profile->dents[i])continue ;
@@ -408,6 +414,7 @@ int __aa_fs_profile_mkdir(struct aa_profile *profile, struct dentry *parent)if (!parent) {struct aa_profile *p; + p =aa_deref_parent (profile); dent =prof_dir (p);/* adding to parent that previously didn't have children */
@@ -419,6 +426,7 @@ int __aa_fs_profile_mkdir(struct aa_profile *profile, struct dentry *parent)if (!profile->dirname) {int len, id_len; + len =mangle_name (profile->base.name,NULL ); id_len =snprintf (NULL ,0 ,".%ld" , profile->ns->uniq_id);
@@ -662,6 +670,7 @@ static struct aa_profile *next_profile(struct aa_namespace *root, struct aa_profile *profile) {struct aa_profile *next =__next_profile (profile); +if (next)return next;
@@ -683,6 +692,7 @@ static void *p_start(struct seq_file *f, loff_t *pos)struct aa_profile *profile =NULL ;struct aa_namespace *root =aa_current_profile ()->ns;loff_t l = *pos; + f->private =aa_get_namespace (root);/* find the first profile */
Signature · 1 line
2.1.4
PATCH 3/7 security: apparmor: apparmorfs.c Space after cast
In reply to Walt FeaselMake style modifications for:
CHECK: No space is necessary after a cast
--- security/apparmor/apparmorfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
security/apparmor/apparmorfs.c +1 -1
@@ -240,7 +240,7 @@ static int aa_fs_seq_profile_open(struct inode *inode, struct file *file,static int aa_fs_seq_profile_release (struct inode *inode,struct file *file) { -struct seq_file *seq = (struct seq_file *) file->private_data; +struct seq_file *seq = (struct seq_file *)file->private_data;if (seq)aa_put_replacedby (seq->private);
Signature · 1 line
2.1.4
PATCH 4/7 security: apparmor: apparmorfs.c WARN_ON vs BUG_ON
In reply to Walt FeaselMake style modifications for:
CHECK: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
---
Not sure if correct use. Seems if copy_size > alloc_size it is corrected by writing partial data and return an error.
security/apparmor/apparmorfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
security/apparmor/apparmorfs.c +1 -1
@@ -87,7 +87,7 @@ static char *aa_simple_write_to_buffer(int op, const char __user *userbuf, {char *data; -BUG_ON (copy_size > alloc_size); +WARN_ON (copy_size > alloc_size);if (*pos !=0 )/* only writes from pos 0, that is complete writes */
Signature · 1 line
2.1.4
PATCH 5/7 security: apparmor: apparmorfs.c Align parenthesis
In reply to Walt FeaselMake style modifications for:
CHECK: Alignment should match open parenthesis
---
Swaps align for over 80 but get whole line this way
security/apparmor/apparmorfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
security/apparmor/apparmorfs.c +1 -2
@@ -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 */
Signature · 1 line
2.1.4
PATCH 6/7 security: apparmor: apparmorfs.c Comparison to NULL
In reply to Walt FeaselMake style modifications for:
CHECK: Comparison to NULL could be written "!data"
---
My first patch of this type checking if
this would be correct
security/apparmor/apparmorfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
security/apparmor/apparmorfs.c +1 -1
@@ -102,7 +102,7 @@ static char *aa_simple_write_to_buffer(int op, const char __user *userbuf,/* freed by caller to simple_write_to_buffer */ data =kvmalloc (alloc_size); -if (data ==NULL ) +if (!data)return ERR_PTR (-ENOMEM );if (copy_from_user (data, userbuf, copy_size)) {
Signature · 1 line
2.1.4
PATCH 7/7 security: apparmor: apparmorfs.c Line continuations
In reply to Walt FeaselMake style modifications for:
WARNING: Avoid unnecessary line continuations
---
Swaps continuation for over 80 but on same line
security/apparmor/apparmorfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
security/apparmor/apparmorfs.c +1 -2
@@ -790,8 +790,7 @@ static const struct file_operations aa_fs_profiles_fops = {/** Base file system setup **/ static struct aa_fs_entry aa_fs_entry_file[] = { -AA_FS_FILE_STRING ("mask" ,"create read write exec append mmap_exec " \ -"link lock" ), +AA_FS_FILE_STRING ("mask" ,"create read write exec append mmap_exec" ," link lock" ), { } };
Signature · 1 line
2.1.4
cover security: apparmor: apparmorfs.c Checkpatch mods
In reply to Walt FeaselQuoted 10 lines · Walt Feasel
Make checkpatch modifications to include:
Multiple blank lines
Blank line after declarations
Space after castWARN_ONvsBUG_ON
Align parenthesis
Comparison to NULL
Line continuations
Quoted 8 lines
Walt Feasel (7):
security: apparmor:apparmorfs.cMultiple blank lines
security: apparmor:apparmorfs.cBlank line after declarations
security: apparmor:apparmorfs.cSpace after cast
security: apparmor:apparmorfs.cWARN_ONvsBUG_ON
security: apparmor:apparmorfs.cAlign parenthesis
security: apparmor:apparmorfs.cComparison to NULL
security: apparmor:apparmorfs.cLine 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
cover security: apparmor: apparmorfs.c Checkpatch mods
Quoted 20 lines · Greg KH
Quoted 10 lines · Walt Feasel
Make checkpatch modifications to include:
Multiple blank lines
Blank line after declarations
Space after castWARN_ONvsBUG_ON
Align parenthesis
Comparison to NULL
Line continuationsQuoted 8 lines
Walt Feasel (7):
security: apparmor:apparmorfs.cMultiple blank lines
security: apparmor:apparmorfs.cBlank line after declarations
security: apparmor:apparmorfs.cSpace after cast
security: apparmor:apparmorfs.cWARN_ONvsBUG_ON
security: apparmor:apparmorfs.cAlign parenthesis
security: apparmor:apparmorfs.cComparison to NULL
security: apparmor:apparmorfs.cLine continuations
Quoted 6 lines
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.
cover security: apparmor: apparmorfs.c Checkpatch mods
Quoted 27 lines · Walt Feasel
Quoted 20 lines · Greg KH
Quoted 10 lines · Walt Feasel
Make checkpatch modifications to include:
Multiple blank lines
Blank line after declarations
Space after castWARN_ONvsBUG_ON
Align parenthesis
Comparison to NULL
Line continuationsQuoted 8 lines
Walt Feasel (7):
security: apparmor:apparmorfs.cMultiple blank lines
security: apparmor:apparmorfs.cBlank line after declarations
security: apparmor:apparmorfs.cSpace after cast
security: apparmor:apparmorfs.cWARN_ONvsBUG_ON
security: apparmor:apparmorfs.cAlign parenthesis
security: apparmor:apparmorfs.cComparison to NULL
security: apparmor:apparmorfs.cLine continuationsQuoted 6 lines
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
Quoted 11 lines
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
cover security: apparmor: apparmorfs.c Checkpatch mods
Quoted 40 lines · Greg KH
Quoted 27 lines · Walt Feasel
Quoted 20 lines · Greg KH
Quoted 10 lines · Walt Feasel
Make checkpatch modifications to include:
Multiple blank lines
Blank line after declarations
Space after castWARN_ONvsBUG_ON
Align parenthesis
Comparison to NULL
Line continuationsQuoted 8 lines
Walt Feasel (7):
security: apparmor:apparmorfs.cMultiple blank lines
security: apparmor:apparmorfs.cBlank line after declarations
security: apparmor:apparmorfs.cSpace after cast
security: apparmor:apparmorfs.cWARN_ONvsBUG_ON
security: apparmor:apparmorfs.cAlign parenthesis
security: apparmor:apparmorfs.cComparison to NULL
security: apparmor:apparmorfs.cLine continuationsQuoted 6 lines
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
Quoted 11 lines
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.
Quoted 7 lines
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?
cover security: apparmor: apparmorfs.c Checkpatch mods
Quoted 45 lines · Walt Feasel
Quoted 44 lines · Greg KH
Quoted 27 lines · Walt Feasel
Quoted 20 lines · Greg KH
Quoted 10 lines · Walt Feasel
Make checkpatch modifications to include:
Multiple blank lines
Blank line after declarations
Space after castWARN_ONvsBUG_ON
Align parenthesis
Comparison to NULL
Line continuationsQuoted 8 lines
Walt Feasel (7):
security: apparmor:apparmorfs.cMultiple blank lines
security: apparmor:apparmorfs.cBlank line after declarations
security: apparmor:apparmorfs.cSpace after cast
security: apparmor:apparmorfs.cWARN_ONvsBUG_ON
security: apparmor:apparmorfs.cAlign parenthesis
security: apparmor:apparmorfs.cComparison to NULL
security: apparmor:apparmorfs.cLine continuationsQuoted 6 lines
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
Quoted 11 lines
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.
Quoted 5 lines
Quoted 3 lines
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
cover security: apparmor: apparmorfs.c Checkpatch mods
Quoted 8 lines · Greg KH
Quoted 2 lines · Walt Feasel
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
cover security: apparmor: apparmorfs.c Checkpatch mods
Quoted 19 lines · Bj?rn Mork
Quoted 8 lines · Greg KH
Quoted 2 lines · Walt Feasel
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_ONwithWARN_ONhere?". 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.
Quoted 3 lines
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
Quoted 9 lines
Bj?rn
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20161126/f95e227c/attachment.html