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
