On 12/30/2011 06:58 PM, Kees Cook wrote:
Hi Seth,
On Fri, Dec 30, 2011 at 06:44:16PM -0800, Seth Arnold wrote:
On Thu, Dec 29, 2011 at 4:46 PM, Kees Cook<[email protected]> wrote:
+#include<linux/capability.h>
Why is this header now required? I didn't spot it in the code.
Heh, yeah, I have no idea. I was wondering that myself while reviewing the
code today. ;)
It for the VFS_CAP_FLAGS_MASK define
+#define AA_FS_U64_FILE(name, value) \
+ { name, \
+ .v_type = AA_FS_U64, .v.u64 = value, \
+&aa_fs_seq_file_ops }
I would rather the name and value have () around them in the expansion:
Good idea, yeah. I'll do that.
+#undef AA_FS_STRING_FILE
Only AA_FS_STRING_FILE is undefed here -- why not also include
AA_FS_U64_FILE and AA_FS_BOOLEAN_FILE?
It was my intention to undef them all; I'll fix this.
-static void __init aafs_remove(const char *name)
...
+static void __init aafs_remove(const char *name, struct dentry *dir_dentry)
{
struct dentry *dentry;
- dentry = lookup_one_len(name, aa_fs_dentry, strlen(name));
+ if (!dir_dentry)
+ return;
+
+ dentry = lookup_one_len(name, dir_dentry, strlen(name));
This looks like a bugfix; should that be rolled in separately?
It's actually not, but "diff" made this hard to review by separating the
old function declaration. I added the dir_dentry argument (it was
aa_fs_dentry always before), so it was part of the refactor.
I've actually just finished a version that includes the rlimit directory,
and the patch has gotten large enough now that I'm going to break up into a
few pieces to make it easier to review.
Thanks Kees!
You bet! Thanks for the review. :) I'll have another set up soon.
-Kees
--
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
