Re: [PATCH v2 11/17] xen/hypfs: add getsize() and findentry() callbacks to hypfs_funcs

[Jürgen Groß <jgross@xxxxxxxx>]

On 04.12.20 09:58, Jan Beulich wrote:
> On 01.12.2020 09:21, Juergen Gross wrote:
> > @@ -197,28 +235,12 @@ static struct hypfs_entry *hypfs_get_entry_rel(struct 
> > hypfs_entry_dir *dir,
> >               end = strchr(path, '\0');
> >           name_len = end - path;
> >   
> > -        again = false;
> > +        entry = dir->e.funcs->findentry(dir, path, name_len);
> > +        if ( IS_ERR(entry) || !*end )
> > +            return entry;
> >   
> > -        list_for_each_entry ( entry, &dir->dirlist, list )
> > -        {
> > -            int cmp = strncmp(path, entry->name, name_len);
> > -            struct hypfs_entry_dir *d = container_of(entry,
> > -                                                     struct hypfs_entry_dir, 
> > e);
> > -
> > -            if ( cmp < 0 )
> > -                return ERR_PTR(-ENOENT);
> > -            if ( !cmp && strlen(entry->name) == name_len )
> > -            {
> > -                if ( !*end )
> > -                    return entry;
> > -
> > -                again = true;
> > -                dir = d;
> > -                path = end + 1;
> > -
> > -                break;
> > -            }
> > -        }
> > +        path = end + 1;
> > +        dir = container_of(entry, struct hypfs_entry_dir, e);
> >       }
> Looking at patch 15 my understanding is that "dir" may get invalidated
> by the call to the ->findentry() hook above. That is, use of dir has
> to be avoided between the two calls. This isn't at all obvious, so I
> wonder whether at least a comment wouldn't want adding to avoid future
> mistakes.
Will add a comment.

> And of course the same comment applies to the IS_ERR() use here vs NULL
> coming back that I already gave for the ->enter() call site.
I'll add an ASSERT().


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature