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

[Gao Xiang <hsiangkao@linux.alibaba.com>]

On 2023/6/9 16:50, Guo Xuenan via Linux-erofs wrote:
> refactor compressor code using constructor.
> 
> Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
> ---
>   lib/compressor.c         | 49 ++++++++++++++++++++++++++++++++++++++++
>   lib/compressor.h         | 15 ++++++++++++
>   lib/compressor_liblzma.c |  5 ++++
>   lib/compressor_lz4.c     |  5 ++++
>   lib/compressor_lz4hc.c   |  5 ++++
>   5 files changed, 79 insertions(+)
> 
> diff --git a/lib/compressor.c b/lib/compressor.c
> index 52eb761..0fa7105 100644
> --- a/lib/compressor.c
> +++ b/lib/compressor.c
> @@ -22,6 +22,40 @@ static const struct erofs_compressor *compressors[] = {
>   #endif
>   };
>   
> +/* compressing configuration specified by users */
> +static struct erofs_supported_algothrim {
> +	int algtype;
> +	const char *name;
> +} erofs_supported_algothrims[] = {
> +	{ Z_EROFS_COMPRESSION_LZ4, "lz4"},
> +	{ Z_EROFS_COMPRESSION_LZ4, "lz4hc"},
> +	{ Z_EROFS_COMPRESSION_LZMA, "lzma"},
> +};
> +
> +static struct erofs_compressors_cfg erofs_ccfg;
> +
> +int erofs_compressor_num(void)
> +{
> +	return erofs_ccfg.erofs_ccfg_num;



> +}
> +
> +void erofs_compressor_register(const char *name, const struct erofs_compressor *alg)
> +{
> +	int i;
> +
> +	for (i = 0; i < erofs_compressor_num(); i++) {
> +		if (!strcmp(erofs_ccfg.compressors[i].name, name)) {
> +			erofs_ccfg.compressors[i].handle.alg = alg;

should we return error?  and also, why we need dynamicly
register algorithms? liberofs expects to have given
algorithms all the time...

> +			return;
> +		}
> +	}
> +
> +	erofs_ccfg.compressors[i].name = name;
> +	erofs_ccfg.compressors[i].handle.alg = alg;
> +	erofs_ccfg.compressors[i].algorithmtype = erofs_supported_algothrims[i].algtype;
> +	erofs_ccfg.erofs_ccfg_num = ++i;
> +}
> +
>   int erofs_compress_destsize(const struct erofs_compress *c,
>   			    const void *src, unsigned int *srcsize,
>   			    void *dst, unsigned int dstsize, bool inblocks)
> @@ -106,3 +140,18 @@ int erofs_compressor_exit(struct erofs_compress *c)
>   		return c->alg->exit(c);
>   	return 0;
>   }
> +
> +void __attribute__((constructor(101))) __erofs_compressor_init(void)


Honestly I don't like __attribute__((constructor)) and
__attribute__((destructor)) which could causes unexpected behaviors and
not good at compatiability.

Thanks,
Gao Xiang