[PATCH v2 1/4] erofs-utils: lib: refactor erofs compressors init

[Guo Xuenan]

On 2023/6/15 18:44, Gao Xiang wrote:
>
>
> On 2023/6/15 18:17, Guo Xuenan via Linux-erofs wrote:
>> using struct erofs_compressors_cfg for erofs compressor
>> global configuration.
>>
>> Signed-off-by: Guo Xuenan <guoxuenan at huawei.com>
>> ---
>>   lib/compressor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/compressor.h | 14 ++++++++++
>>   2 files changed, 82 insertions(+)
>>
>> diff --git a/lib/compressor.c b/lib/compressor.c
>> index 52eb761..88a2fb0 100644
>> --- a/lib/compressor.c
>> +++ b/lib/compressor.c
>> @@ -22,6 +22,74 @@ static const struct erofs_compressor 
>> *compressors[] = {
>>   #endif
>>   };
>>   +/* for compressors type configuration */
>
> This comment might be unnecessary.
>
OKay
>> +static struct erofs_supported_algothrim {
>
>                 ^ algorithm
>
>
>
>
>> +    int algtype;
>> +    const char *name;
>> +} erofs_supported_algothrims[] = {
>> +    { Z_EROFS_COMPRESSION_LZ4, "lz4"},
>> +    { Z_EROFS_COMPRESSION_LZ4, "lz4hc"},
>> +    { Z_EROFS_COMPRESSION_LZMA, "lzma"},
>> +};
>> +
>> +/* global compressors configuration */
>
>
> Let's avoid this comment as well.
>
>
>> +static struct erofs_compressors_cfg erofs_ccfg;
>> +
>> +static void erofs_init_compressor(struct compressor *compressor,
>> +    const struct erofs_compressor *alg)
>> +{
>> +
>> +    compressor->handle.alg = alg;
>> +
>> +    /* should be written in "minimum compression ratio * 100" */
>> +    compressor->handle.compress_threshold = 100;
>> +
>> +    /* optimize for 4k size page */
>> +    compressor->handle.destsize_alignsize = erofs_blksiz();
>> +    compressor->handle.destsize_redzone_begin = erofs_blksiz() - 16;
>> +    compressor->handle.destsize_redzone_end = 
>> EROFS_CONFIG_COMPR_DEF_BOUNDARY;
>> +
>> +    if (alg && alg->init)
>> +        alg->init(&compressor->handle);
>> +}
>> +
>> +static void erofs_compressor_register(const char *name, const struct 
>> erofs_compressor *alg)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < erofs_ccfg.erofs_ccfg_num; i++) {
>> +        if (!strcmp(erofs_ccfg.compressors[i].name, name)) {
>> + erofs_init_compressor(&erofs_ccfg.compressors[i], alg);
>> +            return;
>> +        }
>> +    }
>> +
>> +    erofs_ccfg.compressors[i].name = name;
>> +    erofs_ccfg.compressors[i].algorithmtype = 
>> erofs_supported_algothrims[i].algtype;
>> +    erofs_init_compressor(&erofs_ccfg.compressors[i], alg);
>> +    erofs_ccfg.erofs_ccfg_num = ++i;
>> +}
>> +
>> +void erofs_register_compressors(void)
>> +{
>> +    int i;
>> +
>> +    erofs_ccfg.erofs_ccfg_num = 0;
>> +    for (i = 0; i < ARRAY_SIZE(erofs_supported_algothrims); i++) {
>> + erofs_compressor_register(erofs_supported_algothrims[i].name, NULL);
>> +    }
>> +
>> +#if LZ4_ENABLED
>> +    erofs_compressor_register("lz4", &erofs_compressor_lz4);
>> +#if LZ4HC_ENABLED
>> +    erofs_compressor_register("lz4hc", &erofs_compressor_lz4hc);
>> +#endif
>> +#endif
>> +#if HAVE_LIBLZMA
>> +    erofs_compressor_register("lzma", &erofs_compressor_lzma);
>> +#endif
>> +}
>> +
>>   int erofs_compress_destsize(const struct erofs_compress *c,
>>                   const void *src, unsigned int *srcsize,
>>                   void *dst, unsigned int dstsize, bool inblocks)
>> diff --git a/lib/compressor.h b/lib/compressor.h
>> index cf063f1..1e760b6 100644
>> --- a/lib/compressor.h
>> +++ b/lib/compressor.h
>> @@ -8,6 +8,7 @@
>>   #define __EROFS_LIB_COMPRESSOR_H
>>     #include "erofs/defs.h"
>> +#include "erofs/config.h"
>>     struct erofs_compress;
>>   @@ -40,6 +41,18 @@ struct erofs_compress {
>>       void *private_data;
>>   };
>>   +struct compressor {
>> +    const char *name;
>> +    struct erofs_compress handle;
>> +    unsigned int algorithmtype;
>> +    bool enable;
>
> could we just use a struct erofs_supported_algothrim * to replace
> name and algorithmtype?
>
Yes, of course, your suggestion is better!
> Also, please use `struct erofs_compressor` here.
struct erofs_compressor has been used  lib/compressor.h:15
> Thanks,
> Gao Xiang
>
-- 
Best regards
Guo Xuenan