On Tue, Aug 24, 2021 at 10:58:19AM +0300, Dan Carpenter wrote:
> Hello Konstantin Komarov,
>
> The patch 82cae269cfa9: "fs/ntfs3: Add initialization of super block"
> from Aug 13, 2021, leads to the following
> Smatch static checker warning:
>
> fs/ntfs3/index.c:238 bmp_buf_get()
> warn: 'bh' could be an error pointer
>
> fs/ntfs3/index.c
> 229 data_size = le64_to_cpu(b->nres.data_size);
> 230 if (WARN_ON(off >= data_size)) {
> 231 /* looks like filesystem error */
> 232 return -EINVAL;
> 233 }
> 234
> 235 valid_size = le64_to_cpu(b->nres.valid_size);
> 236
> 237 bh = ntfs_bread_run(sbi, &indx->bitmap_run, off);
> --> 238 if (!bh)
> 239 return -EIO;
> 240
> 241 if (IS_ERR(bh))
>
> This is not a bug, but it is wrong style. When a function returns both
> error pointers and NULL then the NULL return is means the feature is
> disabled. It's not an error. Just that the feature is turned off
> deliberately in the Kconfig or whatever. Don't print an error message,
> just continue with the feature disabled as the admin has requested.
>
> But here NULL is just an error. The ntfs_bread_run() should do:
Agreed.
>
> bh = ntfs_bread();
> if (!bh)
> return ERR_PTR(-EIO);
> return bh;
Can also be like below but probably your version is more readable.
return ntfs_bread(sb, lbo >> sb->s_blocksize_bits) ? : ERR_PTR(-EIO);
>
> 242 return PTR_ERR(bh);
> 243
> 244 bbuf->bh = bh;
> 245
>
> regards,
> dan carpenter
