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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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
