Re: [RESEND] f2fs: add sanity compress level check for compressed file

[Chao Yu]

On 2023/4/3 21:33, Yangtao Li wrote:
> Hi Chao,
>
> > Why not zstd_max_clevel()?
>
> zstd_max_clevel() is only defined when CONFIG_F2FS_FS_ZSTD is enabled,
> using zstd_max_clevel() will result in compile errors otherwise.
>
> If using the following code,
>
> ----------------------------------------------------------------------------
>          switch (ri->i_compress_algorithm) {
>          case COMPRESS_LZO:
>          case COMPRESS_LZORLE:
>                  if (compress_level)
>                          goto err;
>                  break;
>          case COMPRESS_LZ4:
>                  if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
>                                  compress_level > LZ4HC_MAX_CLEVEL)
>                          goto err;
>                  break;
> #ifdef CONFIG_F2FS_FS_ZSTD
>          case COMPRESS_ZSTD:

Hi Yangtao,

How about:

#ifdef CONFIG_F2FS_FS_ZSTD
	if (!compress_level || compress_level > zstd_max_clevel())
		goto err;
#endif
	break;

>                  if (!compress_level || compress_level > zstd_max_clevel())
>                          goto err;
>                  break;
> #endif
>          default:
>                  goto err;
>          }
> ----------------------------------------------------------------------------
>
> then we will get this result:
>
> 	F2FS-fs (loop0): sanity_check_compress_inode: inode (ino=4) has
> 			unsupported compress level: 0, run fsck to fix
>
> Another way is to use the following code, which ignores the check for
> level when CONFIG_F2FS_FS_ZSTD is not enabled.
>
> ----------------------------------------------------------------------------
>          switch (ri->i_compress_algorithm) {
>          case COMPRESS_LZO:
>          case COMPRESS_LZORLE:
>                  if (compress_level)
>                          goto err;
>                  break;
>          case COMPRESS_LZ4:
>                  if ((compress_level && compress_level < LZ4HC_MIN_CLEVEL) ||
>                                  compress_level > LZ4HC_MAX_CLEVEL)
>                          goto err;
>                  break;
>          case COMPRESS_ZSTD:
> #ifdef CONFIG_F2FS_FS_ZSTD
>                  if (!compress_level || compress_level > zstd_max_clevel())
>                          goto err;
>                  break;
> #else
>                  return true;
> #endif
>          default:
>                  goto err;
>          }
> ----------------------------------------------------------------------------
>
> Perhaps exporting ZSTD_MAX_CLEVEL is a better choice?
>
> Thx,
> Yangtao