Re: [PATCH v2 01/15] VFS: note error returns is documentation for various lookup functions

[Chris Mason <clm@meta.com>]

NeilBrown <neilb@ownmail.net> wrote:
> From: NeilBrown <neil@brown.name>
> 
> Darrick recently noted that try_lookup_noperm() is documented as
> "Look up a dentry by name in the dcache, returning NULL if it does not
> currently exist." but it can in fact return an error.
> 
> So update the documentation for that and related function.
>

Hi everyone,

I don't normally forward the typos, but since this is a documentation-y patch:

commit 0254b9b974f23889898562aa94f6428bf30eb6b5
Author: NeilBrown <neil@brown.name>

VFS: note error returns is documentation for various lookup functions
                       ^^^^^ in?

Darrick recently noted that try_lookup_noperm() is documented as
"Look up a dentry by name in the dcache, returning NULL if it does not
currently exist." but it can in fact return an error. So update the
documentation for that and related function.

Link: https://lore.kernel.org/all/20260218234917.GA6490@frogsfrogsfrogs/

> diff --git a/fs/namei.c b/fs/namei.c
> index 58f715f7657e2..e4ac07a4090e7 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3124,7 +3124,8 @@ static int lookup_one_common(struct mnt_idmap *idmap,
>   * @base:	base directory to lookup from
>   *
>   * Look up a dentry by name in the dcache, returning NULL if it does not
> - * currently exist.  The function does not try to create a dentry and if one
> + * currently exist or an error is there is a problem with the name.
                                       ^^

There's a grammar error: "is there is" should be "if there is".

> + * The function does not try to create a dentry and if one
>   * is found it doesn't try to revalidate it.
>   *
>   * Note that this routine is purely a helper for filesystem usage and should
> @@ -3132,6 +3133,11 @@ static int lookup_one_common(struct mnt_idmap *idmap,
>   *
>   * No locks need be held - only a counted reference to @base is needed.
>   *
> + * Returns:
> + *   - ref-counted dentry on success, or
> + *   - %NULL if name could not be found, or
> + *   - ERR_PTR(-EACCES) if name is dot or dotdot or contains a slash or nul, or
> + *   - ERR_PTR() if fs provide ->d_hash, and this returned an error.
>   */
>  struct dentry *try_lookup_noperm(struct qstr *name, struct dentry *base)
>  {

[ ... ]

> @@ -3208,6 +3214,11 @@ EXPORT_SYMBOL(lookup_one);
>   *
>   * Unlike lookup_one, it should be called without the parent
>   * i_rwsem held, and will take the i_rwsem itself if necessary.
> + *
> + * Returns:= A dentry, possibly negative, or
           ^^^

There's a typo: "Returns:=" should be "Returns:".

> + *	   - same errors as try_lookup_noperm() or
> + *	   - ERR_PTR(-ENOENT) if parent has been removed, or
> + *	   - ERR_PTR(-EACCES) if parent directory is not searchable.
>   */
>  struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, struct qstr *name,
>  				   struct dentry *base)

[ ... ]

> @@ -3244,6 +3255,10 @@ EXPORT_SYMBOL(lookup_one_unlocked);
>   * It should be called without the parent i_rwsem held, and will take
>   * the i_rwsem itself if necessary.  If a fatal signal is pending or
>   * delivered, it will return %-EINTR if the lock is needed.
> + *
> + * Returns: A dentry, possibly negative, or
> + *	   - same errors as lookup_one_unlocked() or
> + *	   - ERR_PTR(-EINTR) is a fatal signal is pending.
                             ^^

There's a grammar error: "is a fatal signal is pending" should be "if a
fatal signal is pending".

> + */
>  struct dentry *lookup_one_positive_killable(struct mnt_idmap *idmap,
>  					    struct qstr *name,