[PATCH] cifs: Clean up an error code in cifs_root_iget()

[Steve French]

Won't that change behavior and cause the success case (server returned
0, and does support POSIX and &inode pointer is ok) to now return
-EINVAL.

Before if rc was 0 and inode was filled in we wouldn't execute this else block:

    } else if (rc) {
        iget_failed(inode);
        inode = ERR_PTR(rc);
    }

But with your change, won't we return EINVAL for the success case?

How about just changing:

iget_no_retry:
    if (!inode) {
        inode = ERR_PTR(rc);
        goto out;
    }

to

iget_no_retry:
    if (!inode) {
        if (rc == 0)
              rc = -EINVAL;
        inode = ERR_PTR(rc);
        goto out;
    }



On Wed, Feb 27, 2019 at 11:28 PM Dan Carpenter via samba-technical
<samba-technical at lists.samba.org> wrote:
>
> This patch silences a Smatch warning:
>
> fs/cifs/inode.c:1094 cifs_root_iget() warn: passing zero to 'ERR_PTR'
>
> The shouldn't have a noticeable effect on runtime, it's basically
> a cleanup.  The code is checking to ensure that cifs_get_inode_info_unix()
> returns -EOPNOTSUPP when we have the UNIX extensions.  From the patch
> description in commit b5b374eab11e ("Workaround Mac server problem")
> this affects Macs.
>
> Presumably most of the time "rc" is zero, which means we return
> ERR_PTR(0) which is NULL.  This cifs_root_iget() function is only called
> from cifs_read_super() and if we return NULL that causes d_make_root()
> to return NULL so in the end we fail with -ENOMEM.
>
> After this patch is applied we instead return with -EINVAL.
>
> Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
> ---
>  fs/cifs/inode.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 0f53ecb071ac..e40c554bb2f3 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1080,8 +1080,10 @@ struct inode *cifs_root_iget(struct super_block *sb)
>         if (tcon->unix_ext) {
>                 rc = cifs_get_inode_info_unix(&inode, path, sb, xid);
>                 /* some servers mistakenly claim POSIX support */
> -               if (rc != -EOPNOTSUPP)
> +               if (rc != -EOPNOTSUPP) {
> +                       rc = -EINVAL;
>                         goto iget_no_retry;
> +               }
>                 cifs_dbg(VFS, "server does not support POSIX extensions");
>                 tcon->unix_ext = false;
>         }
> --
> 2.17.1
>
>


--
Thanks,

Steve