Re: [PATCH 4/6 v2] autofs4: factor should_expire() out of autofs4_expire_indirect.

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

On Wed, 16 Jul 2014 15:50:17 +0800 Ian Kent <raven@themaw.net> wrote:


> > +	if (simple_empty(dentry))
> > +		return NULL;
> > +
> > +	/* Case 2: tree mount, expire iff entire tree is not busy */
> > +	if (!exp_leaves) {
> > +		/* Path walk currently on this dentry? */
> > +		ino_count = atomic_read(&ino->count) + 1;
> > +		if (d_count(dentry) > ino_count)
> > +			return NULL;
> > +
> > +		if (!autofs4_tree_busy(mnt, dentry, timeout, do_now))
> > +			return dentry;
> > +		/*
> > +		 * Case 3: pseudo direct mount, expire individual leaves
> > +		 *	   (autofs-4.1).
> > +		 */
> 
> I recommend removing one of the tab stops on this since it relates to
> the else case but feels like it's "attached" to the code above the else.

The auto-reindent feature of emacs put it there and I didn't double check.
I've fix it.


> 
> > +	} else {
> > +		/* Path walk currently on this dentry? */
> > +		struct dentry *expired;
> > +		ino_count = atomic_read(&ino->count) + 1;
> > +		if (d_count(dentry) > ino_count)
> > +			return NULL;
> > +
> > +		expired = autofs4_check_leaves(mnt, dentry, timeout, do_now);
> > +		if (expired) {
> > +			if (expired == dentry)
> > +				dput(dentry);
> > +			return dentry;
> 
> Umm .. I think this should be "return expired".

Yes, of course it should.  Fixed.

> 
> Otherwise this looks like a straight refactor.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]