[bug report] erofs: enable error reporting for z_erofs_fixup_insize()

[Gao Xiang]

Hi Dan,

On 2025/12/2 16:32, Dan Carpenter wrote:
> Hello Gao Xiang,
> 
> Commit 30e13e41a0eb ("erofs: enable error reporting for
> z_erofs_fixup_insize()") from Nov 27, 2025 (linux-next), leads to the
> following Smatch static checker warning:
> 
> fs/erofs/decompressor.c:217 z_erofs_lz4_decompress_mem() warn: 'reason' isn't an ERR_PTR
> fs/erofs/decompressor_lzma.c:193 z_erofs_lzma_decompress() warn: 'reason' is an error pointer or valid
> fs/erofs/decompressor_zstd.c:180 z_erofs_zstd_decompress() warn: 'reason' is an error pointer or valid
> 
> fs/erofs/decompressor.c
>      198 static int z_erofs_lz4_decompress_mem(struct z_erofs_decompress_req *rq, u8 *dst)
>      199 {
>      200         bool support_0padding = false, may_inplace = false;
>      201         unsigned int inputmargin;
>      202         u8 *out, *headpage, *src;
>      203         const char *reason;
>      204         int ret, maptype;
>      205
>      206         DBG_BUGON(*rq->in == NULL);
>      207         headpage = kmap_local_page(*rq->in);
>      208
>      209         /* LZ4 decompression inplace is only safe if zero_padding is enabled */
>      210         if (erofs_sb_has_zero_padding(EROFS_SB(rq->sb))) {
>      211                 support_0padding = true;
>      212                 reason = z_erofs_fixup_insize(rq, headpage + rq->pageofs_in,
>      213                                 min_t(unsigned int, rq->inputsize,
>      214                                       rq->sb->s_blocksize - rq->pageofs_in));
>      215                 if (reason) {
>      216                         kunmap_local(headpage);
> --> 217                         return IS_ERR(reason) ? PTR_ERR(reason) : -EFSCORRUPTED;
>      218                 }
> 
> The z_erofs_fixup_insize() function used to return error pointers, but
> now it returns an error string or NULL.  So probably we could just
> change this to:
> 
> 		return -EFSCORRUPTED;

I think `return -EFSCORRUPTED;` is valid for now, but later
z_erofs_lz4_decompress_mem() will be changed into returning
`const char *` as well but it's late for this cycle.

> 
> Are we planning to make it return error pointers again?

z_erofs_lz4_decompress_mem() will be `const char *` later too
along with other cleanups, but it's somewhat late for this cycle.

> 
> NULL means success in this case, right?  It's sort of weird how NULL means
> success and a valid pointer means failure.

Because -EFSCORRUPTED isn't enough to represent the detailed
decompression errors (because each decompressor has its own
error messages and it's helpful for users to know how the
compressed data is corrupted).

NULL means success, and both valid pointers or error pointers
!= NULL means failure, so personally I don't think it's weird.

A valid pointer indicates an detailed error string to be
printed so that users know the detailed reasons and the error
number is -EFSCORRUPTED.

Thanks,
Gao Xiang