Re: nfs4_show_superblock considered harmful :-)

["J. Bruce Fields" <bfields@fieldses.org>]

On Fri, Jul 17, 2020 at 11:31:17AM +1000, NeilBrown wrote:
> On Thu, Jul 16 2020, J. Bruce Fields wrote:
> 
> > On Fri, Jul 17, 2020 at 09:43:40AM +1000, NeilBrown wrote:
> >> On Thu, Jul 16 2020, J. Bruce Fields wrote:
> >> > --- a/fs/nfsd/nfs4state.c
> >> > +++ b/fs/nfsd/nfs4state.c
> >> > @@ -507,6 +507,16 @@ find_any_file(struct nfs4_file *f)
> >> >  	return ret;
> >> >  }
> >> >  
> >> > +static struct nfsd_file *find_deleg_file(struct nfs4_file *f)
> >> > +{
> >> > +	struct nfsd_file *ret;
> >> > +
> >> > +	spin_lock(&f->fi_lock);
> >> > +	ret = nfsd_file_get(f->fi_deleg_file);
> >> 
> >> A test on f->fi_deleg_file being non-NULL would make this look safer.
> >> It would  also make the subsequent test on the return value appear sane.
> >
> > Yes, thanks!-b.
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index c2a2e56c896d..6e8811e7c134 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -509,10 +509,11 @@ find_any_file(struct nfs4_file *f)
> >  
> >  static struct nfsd_file *find_deleg_file(struct nfs4_file *f)
> >  {
> > -	struct nfsd_file *ret;
> > +	struct nfsd_file *ret = NULL;
> >  
> >  	spin_lock(&f->fi_lock);
> > -	ret = nfsd_file_get(f->fi_deleg_file);
> > +	if (f->fi_deleg_file)
> > +		ret = nfsd_file_get(f->fi_deleg_file);
> >  	spin_unlock(&f->fi_lock);
> >  	return ret;
> >  }
> 
> Reviewed-by: NeilBrown <neilb@suse.de>
> 
> for the whole patch.

Thanks!--b.