On Tue, 2 Oct 2012 11:53:38 -0400
Christoph Hellwig <hch(a)infradead.org> wrote:
 On Mon, Oct 01, 2012 at 08:16:11PM -0400, Jeff Layton wrote:
 > In some cases, we were passing in NULL even when we have a dentry.
 > 
 > Reported-by: Eric Paris <eparis(a)redhat.com>
 > Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
 > ---
 >  kernel/auditsc.c | 4 ++--
 >  1 file changed, 2 insertions(+), 2 deletions(-)
 > 
 > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
 > index 4b96415..5c45b9b 100644
 > --- a/kernel/auditsc.c
 > +++ b/kernel/auditsc.c
 > @@ -2226,7 +2226,7 @@ void __audit_inode_child(const struct dentry *dentry,
 >  		if (!strcmp(dname, n->name) ||
 >  		     !audit_compare_dname_path(dname, n->name, &dirlen)) {
 >  			if (inode)
 > -				audit_copy_inode(n, NULL, inode);
 > +				audit_copy_inode(n, dentry, inode);
 
 Btw, the calling conventions here also seems fairly ugly.
 
 Instead of the optional dentry parameter I'd have a audit_copy_inode
 that takes just the name and the inode, and an optional direct  call
 to audit_copy_fcaps for those callers that have a dentry.  That would
 also allow removing the branch for the dentry == NULL case in
 audit_copy_fcaps.
  
[...]
 On Mon, Oct 01, 2012 at 08:16:12PM -0400, Jeff Layton wrote:
 > If name is NULL then the condition in the loop will never be true. Also,
 > with this change, we can eliminate the check for n->name == NULL since
 > the equivalence check will never be true if it is.
 
 Given that name == NULL is a static condition it seems like these
 should be two different calls, E.g. audit_dentry and audit_path.
  
Thanks Christoph,
Both of the above are good suggestions, but I'd prefer not to have to
respin this whole set to implement them. I believe what I have here is
an improvement on what's there now. Do you have any objection to doing
the above in a separate set on top of this series?
-- 
Jeff Layton <jlayton(a)redhat.com>