erofs-utils: mkfs: reorganize logic in erofs_compressor_init()

Currently, the initialization of compressors follows an unusual order:
`.init()` is called first, followed by `.setlevel()`, and then
`.setdictsize()`.  However, the actual initialization occurs within the
last-called `.setdictsize()`, for the MicroLZMA, DEFLATE, and libdeflate
compressors.

This patch reorders these functions, with `.init()` now being invoked
last, allowing it to use the compression level and dictsize already set
so that the behavior of the functions matches their names.

Signed-off-by: Yifan Zhao <zhaoyifan@sjtu.edu.cn>
Link: https://lore.kernel.org/r/20240121122902.207756-1-zhaoyifan@sjtu.edu.cn
[ Gao Xiang: refine the commit message. ]
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

diff --git a/lib/compressor.c b/lib/compressor.c
index 5321a92dfd084e37e3b562032ecfc585da219082..4720e72d7609fdeb39c6c79a25bc4f1f5a3edbd8 100644 (file)
--- a/lib/compressor.c
+++ b/lib/compressor.c
@@ -100,10 +100,6 @@ int erofs_compressor_init(struct erofs_sb_info *sbi, struct erofs_compress *c,
                if (!erofs_algs[i].c)
                        continue;
 
-               ret = erofs_algs[i].c->init(c);
-               if (ret)
-                       return ret;
-
                if (erofs_algs[i].c->setlevel) {
                        ret = erofs_algs[i].c->setlevel(c, compression_level);
                        if (ret) {
@@ -130,6 +126,10 @@ int erofs_compressor_init(struct erofs_sb_info *sbi, struct erofs_compress *c,
                        return -EINVAL;
                }
 
+               ret = erofs_algs[i].c->init(c);
+               if (ret)
+                       return ret;
+
                if (!ret) {
                        c->alg = &erofs_algs[i];
                        return 0;

diff --git a/lib/compressor_deflate.c b/lib/compressor_deflate.c
index e2f19099e78be864c29f1e6b50f01f6778aba9bf..8629415efe62a6152d100796270de9eb5f493484 100644 (file)
--- a/lib/compressor_deflate.c
+++ b/lib/compressor_deflate.c
@@ -36,7 +36,13 @@ static int compressor_deflate_exit(struct erofs_compress *c)
 
 static int compressor_deflate_init(struct erofs_compress *c)
 {
-       c->private_data = NULL;
+       if (c->private_data) {
+               kite_deflate_end(c->private_data);
+               c->private_data = NULL;
+       }
+       c->private_data = kite_deflate_init(c->compression_level, c->dict_size);
+       if (IS_ERR_VALUE(c->private_data))
+               return PTR_ERR(c->private_data);
 
        erofs_warn("EXPERIMENTAL DEFLATE algorithm in use. Use at your own risk!");
        erofs_warn("*Carefully* check filesystem data correctness to avoid corruption!");
@@ -50,6 +56,10 @@ static int erofs_compressor_deflate_setlevel(struct erofs_compress *c,
        if (compression_level < 0)
                compression_level = erofs_compressor_deflate.default_level;
 
+       if (compression_level > erofs_compressor_deflate.best_level) {
+               erofs_err("invalid compression level %d", compression_level);
+               return -EINVAL;
+       }
        c->compression_level = compression_level;
        return 0;
 }
@@ -57,26 +67,13 @@ static int erofs_compressor_deflate_setlevel(struct erofs_compress *c,
 static int erofs_compressor_deflate_setdictsize(struct erofs_compress *c,
                                                u32 dict_size)
 {
-       void *s;
-
-       if (c->private_data) {
-               kite_deflate_end(c->private_data);
-               c->private_data = NULL;
-       }
+       if (!dict_size)
+               dict_size = erofs_compressor_deflate.default_dictsize;
 
        if (dict_size > erofs_compressor_deflate.max_dictsize) {
-               erofs_err("dict size %u is too large", dict_size);
+               erofs_err("dictionary size %u is too large", dict_size);
                return -EINVAL;
        }
-
-       if (!dict_size)
-               dict_size = erofs_compressor_deflate.default_dictsize;
-
-       s = kite_deflate_init(c->compression_level, dict_size);
-       if (IS_ERR(s))
-               return PTR_ERR(s);
-
-       c->private_data = s;
        c->dict_size = dict_size;
        return 0;
 }

diff --git a/lib/compressor_libdeflate.c b/lib/compressor_libdeflate.c
index c0b019a96ceb6d32be924abbc53bb29e8405a55a..62d93f75b94c8d21a5e063a2e0eb888f3e654856 100644 (file)
--- a/lib/compressor_libdeflate.c
+++ b/lib/compressor_libdeflate.c
@@ -82,7 +82,10 @@ static int compressor_libdeflate_exit(struct erofs_compress *c)
 
 static int compressor_libdeflate_init(struct erofs_compress *c)
 {
-       c->private_data = NULL;
+       libdeflate_free_compressor(c->private_data);
+       c->private_data = libdeflate_alloc_compressor(c->compression_level);
+       if (!c->private_data)
+               return -ENOMEM;
 
        erofs_warn("EXPERIMENTAL libdeflate compressor in use. Use at your own risk!");
        return 0;
@@ -94,10 +97,10 @@ static int erofs_compressor_libdeflate_setlevel(struct erofs_compress *c,
        if (compression_level < 0)
                compression_level = erofs_compressor_deflate.default_level;
 
-       libdeflate_free_compressor(c->private_data);
-       c->private_data = libdeflate_alloc_compressor(compression_level);
-       if (!c->private_data)
-               return -ENOMEM;
+       if (compression_level > erofs_compressor_deflate.best_level) {
+               erofs_err("invalid compression level %d", compression_level);
+               return -EINVAL;
+       }
        c->compression_level = compression_level;
        return 0;
 }

diff --git a/lib/compressor_liblzma.c b/lib/compressor_liblzma.c
index 57d2eb93730eafe259e76d71069b8d12cdf3c120..7183b0b29d64b2f5f70cf2c81f033a6953abb4bb 100644 (file)
--- a/lib/compressor_liblzma.c
+++ b/lib/compressor_liblzma.c
@@ -55,19 +55,13 @@ static int erofs_compressor_liblzma_exit(struct erofs_compress *c)
 static int erofs_compressor_liblzma_setlevel(struct erofs_compress *c,
                                             int compression_level)
 {
-       struct erofs_liblzma_context *ctx = c->private_data;
-       u32 preset;
-
        if (compression_level < 0)
-               preset = LZMA_PRESET_DEFAULT;
-       else if (compression_level >= 100)
-               preset = (compression_level - 100) | LZMA_PRESET_EXTREME;
-       else
-               preset = compression_level;
+               compression_level = erofs_compressor_lzma.default_level;
 
-       if (lzma_lzma_preset(&ctx->opt, preset))
+       if (compression_level > erofs_compressor_lzma.best_level) {
+               erofs_err("invalid compression level %d", compression_level);
                return -EINVAL;
-
+       }
        c->compression_level = compression_level;
        return 0;
 }
@@ -75,18 +69,14 @@ static int erofs_compressor_liblzma_setlevel(struct erofs_compress *c,
 static int erofs_compressor_liblzma_setdictsize(struct erofs_compress *c,
                                                u32 dict_size)
 {
-       struct erofs_liblzma_context *ctx = c->private_data;
+       if (!dict_size)
+               dict_size = erofs_compressor_lzma.default_dictsize;
 
        if (dict_size > erofs_compressor_lzma.max_dictsize ||
            dict_size < 4096) {
-               erofs_err("invalid dict size %u", dict_size);
+               erofs_err("invalid dictionary size %u", dict_size);
                return -EINVAL;
        }
-
-       if (!dict_size)
-               dict_size = erofs_compressor_lzma.default_dictsize;
-
-       ctx->opt.dict_size = dict_size;
        c->dict_size = dict_size;
        return 0;
 }
@@ -94,11 +84,24 @@ static int erofs_compressor_liblzma_setdictsize(struct erofs_compress *c,
 static int erofs_compressor_liblzma_init(struct erofs_compress *c)
 {
        struct erofs_liblzma_context *ctx;
+       u32 preset;
 
        ctx = malloc(sizeof(*ctx));
        if (!ctx)
                return -ENOMEM;
        ctx->strm = (lzma_stream)LZMA_STREAM_INIT;
+
+       if (c->compression_level < 0)
+               preset = LZMA_PRESET_DEFAULT;
+       else if (c->compression_level >= 100)
+               preset = (c->compression_level - 100) | LZMA_PRESET_EXTREME;
+       else
+               preset = c->compression_level;
+
+       if (lzma_lzma_preset(&ctx->opt, preset))
+               return -EINVAL;
+       ctx->opt.dict_size = c->dict_size;
+
        c->private_data = ctx;
        erofs_warn("EXPERIMENTAL MicroLZMA feature in use. Use at your own risk!");
        erofs_warn("Note that it may take more time since the compressor is still single-threaded for now.");

diff --git a/lib/compressor_lz4hc.c b/lib/compressor_lz4hc.c
index b410e155efe3ad53c2514bf55a8d8f6219479858..6fc8847dad4d6add523bd0f0ed0d84642da54826 100644 (file)
--- a/lib/compressor_lz4hc.c
+++ b/lib/compressor_lz4hc.c
@@ -7,6 +7,7 @@
 #define LZ4_HC_STATIC_LINKING_ONLY (1)
 #include <lz4hc.h>
 #include "erofs/internal.h"
+#include "erofs/print.h"
 #include "compressor.h"
 
 #ifndef LZ4_DISTANCE_MAX       /* history window size */
@@ -49,8 +50,10 @@ static int compressor_lz4hc_init(struct erofs_compress *c)
 static int compressor_lz4hc_setlevel(struct erofs_compress *c,
                                     int compression_level)
 {
-       if (compression_level > LZ4HC_CLEVEL_MAX)
+       if (compression_level > erofs_compressor_lz4hc.best_level) {
+               erofs_err("invalid compression level %d", compression_level);
                return -EINVAL;
+       }
 
        c->compression_level = compression_level < 0 ?
                LZ4HC_CLEVEL_DEFAULT : compression_level;