Re: [PATCH 3/4] switch all filesystems over to d_obtain_alias

[Christoph Hellwig <hch@infradead.org>]

On Tue, Sep 09, 2008 at 03:28:14PM +0200, Miklos Szeredi wrote:
> > -	if (get_node_id(inode) != FUSE_ROOT_ID) {
> > +	entry = d_obtain_alias(inode);
> > +	if (entry && !IS_ERR(entry) && get_node_id(inode) != FUSE_ROOT_ID) {
> 
> checking for entry != NULL superfluous here.

True.

> >  	}
> > @@ -696,17 +692,14 @@ static struct dentry *fuse_get_parent(st
> >  	name.name = "..";
> >  	err = fuse_lookup_name(child_inode->i_sb, get_node_id(child_inode),
> >  			       &name, &outarg, &inode);
> > -	if (err && err != -ENOENT)
> > -		return ERR_PTR(err);
> > -	if (err || !inode)
> > -		return ERR_PTR(-ESTALE);
> > -
> > -	parent = d_alloc_anon(inode);
> > -	if (!parent) {
> > -		iput(inode);
> > -		return ERR_PTR(-ENOMEM);
> > +	if (err) {
> > +		if (err == -ENOENT)
> > +			return -ESTALE;
> > +		return err;
> >  	}
> > -	if (get_node_id(inode) != FUSE_ROOT_ID) {
> > +
> > +	parent = d_obtain_alias(inode);
> > +	if (parent && !IS_ERR(parent) && get_node_id(inode) != FUSE_ROOT_ID) {
> 
> This is buggy: we want to return ERR_PTR(-ESTALE) for !inode, not
> NULL. So again, either d_obtain_alias() should turn !inode into
> -ENOENT, (which would gain nothing in this case) or it should simply
> expect inode to be not NULL.

ESTALE doesn't really help us here, but I agree that we want an error
return.

> Doing the !inode check in d_obtain_alias() seems only to confuse
> things, instead of simplifying them.

Doing the !inode check here means we can do a return
d_obtain_alias(foo_iget())) tailcall which is quite useful to reduce the
mess in the export operations.