On Wed, 24 Aug 2022 at 23:35, Tom Talpey <tom@xxxxxxxxxx> wrote:
>
> On 8/23/2022 8:27 PM, Ronnie Sahlberg wrote:
> > Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > ---
> > fs/cifs/cached_dir.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/cifs/cached_dir.c b/fs/cifs/cached_dir.c
> > index c2f5b71a3c9f..77880470c7ea 100644
> > --- a/fs/cifs/cached_dir.c
> > +++ b/fs/cifs/cached_dir.c
> > @@ -47,11 +47,12 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
> > if (cifs_sb->root == NULL)
> > return -ENOENT;
> >
> > + if (!strlen(path))
> > + dentry = cifs_sb->root;
>
> Wouldn't it be safer and more efficient to simply test
> "if (path[0] == 0)"?
Thanks!
Right. I have changed it like so.
>
> But, why would a non-null path ever be passed, if it
> always fails? Seems like a pointless call in the first
> place.
>
> > +
> > if (strlen(path))
>
> Simply "else"? No need to recompute strlen.
Done.
It was done this way because shortly later in the patch series some of
these checks go away
once we add the capability to cache more than just the root directory.
> Tom.
>
> > return -ENOENT;
> >
> > - dentry = cifs_sb->root;
> > -
> > cfid = &tcon->cfids->cfid;
> > mutex_lock(&cfid->fid_mutex);
> > if (cfid->is_valid) {
> > @@ -177,7 +178,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
> > cfid->tcon = tcon;
> > cfid->is_valid = true;
> > cfid->dentry = dentry;
> > - dget(dentry);
> > + if (dentry)
> > + dget(dentry);
> > kref_init(&cfid->refcount);
> >
> > /* BB TBD check to see if oplock level check can be removed below */
