Hi Guifu,

On Wed, Oct 23, 2019 at 12:36:16AM +0800, Li Guifu wrote:
> From: Gao Xiang <[email protected]>
> 
> Users can get knowledge of supported compression
> algorithms then.
> 
> Signed-off-by: Gao Xiang <[email protected]>
> Signed-off-by: Li Guifu <[email protected]>
> ---

That is a good idea, how about adding some changelogs
at this place (you could try this from now on), e.g:

changes since v1:
 - update according to
   https://lore.kernel.org/r/[email protected]

>  include/erofs/compress.h |  2 ++
>  lib/compressor.c         | 23 ++++++++++++++---------
>  lib/compressor.h         |  1 +
>  lib/compressor_lz4.c     |  1 +
>  lib/compressor_lz4hc.c   |  1 +
>  mkfs/main.c              | 15 +++++++++++++++
>  6 files changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/include/erofs/compress.h b/include/erofs/compress.h
> index e0abb8f..fa91873 100644
> --- a/include/erofs/compress.h
> +++ b/include/erofs/compress.h
> @@ -21,5 +21,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode);
>  int z_erofs_compress_init(void);
>  int z_erofs_compress_exit(void);
>  
> +const char *z_erofs_list_available_compressors(unsigned int i);
> +
>  #endif
>  
> diff --git a/lib/compressor.c b/lib/compressor.c
> index 8cc2f43..6502437 100644
> --- a/lib/compressor.c
> +++ b/lib/compressor.c
> @@ -12,6 +12,15 @@
>  
>  #define EROFS_CONFIG_COMPR_DEF_BOUNDARY              (128)
>  
> +static struct erofs_compressor *compressors[] = {
> +#if LZ4_ENABLED
> +#if LZ4HC_ENABLED
> +             &erofs_compressor_lz4hc,
> +#endif
> +             &erofs_compressor_lz4,
> +#endif
> +};
> +
>  int erofs_compress_destsize(struct erofs_compress *c,
>                           int compression_level,
>                           void *src,
> @@ -36,18 +45,14 @@ int erofs_compress_destsize(struct erofs_compress *c,
>       return ret;
>  }
>  
> +const char *z_erofs_list_available_compressors(unsigned int i)
> +{
> +     return (i >= ARRAY_SIZE(compressors)) ? NULL : compressors[i]->name;

no need extra parentheses

        return i >= ARRAY_SIZE(compressors) ? NULL : compressors[i]->name;

> +}
> +
>  int erofs_compressor_init(struct erofs_compress *c,
>                         char *alg_name)
>  {
> -     static struct erofs_compressor *compressors[] = {
> -#if LZ4_ENABLED
> -#if LZ4HC_ENABLED
> -             &erofs_compressor_lz4hc,
> -#endif
> -             &erofs_compressor_lz4,
> -#endif
> -     };
> -
>       int ret, i;
>  
>       /* should be written in "minimum compression ratio * 100" */
> diff --git a/lib/compressor.h b/lib/compressor.h
> index 6429b2a..4b76c4c 100644
> --- a/lib/compressor.h
> +++ b/lib/compressor.h
> @@ -14,6 +14,7 @@
>  struct erofs_compress;
>  
>  struct erofs_compressor {
> +     const char *name;

add a blank line?

>       int default_level;
>       int best_level;
>  
> diff --git a/lib/compressor_lz4.c b/lib/compressor_lz4.c
> index 0d33223..a9cbb80 100644
> --- a/lib/compressor_lz4.c
> +++ b/lib/compressor_lz4.c
> @@ -39,6 +39,7 @@ static int compressor_lz4_init(struct erofs_compress *c,

How about moving
 32 static int compressor_lz4_init(struct erofs_compress *c,
 33                                  char *alg_name)
 34 {
 35         if (alg_name && strcmp(alg_name, "lz4"))
 36                 return -EINVAL;

to compress.c?

>  }
>  
>  struct erofs_compressor erofs_compressor_lz4 = {
> +     .name = "lz4",
>       .default_level = 0,
>       .best_level = 0,
>       .init = compressor_lz4_init,
> diff --git a/lib/compressor_lz4hc.c b/lib/compressor_lz4hc.c
> index 14e0175..a160c1a 100644
> --- a/lib/compressor_lz4hc.c
> +++ b/lib/compressor_lz4hc.c
> @@ -52,6 +52,7 @@ static int compressor_lz4hc_init(struct erofs_compress *c,

ditto.

Thanks,
Gao Xiang

>  }
>  
>  struct erofs_compressor erofs_compressor_lz4hc = {
> +     .name = "lz4hc",
>       .default_level = LZ4HC_CLEVEL_DEFAULT,
>       .best_level = LZ4HC_CLEVEL_MAX,
>       .init = compressor_lz4hc_init,
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 31cf1c2..1161b3f 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -29,6 +29,19 @@ static struct option long_options[] = {
>       {0, 0, 0, 0},
>  };
>  
> +static void print_available_compressors(FILE *f, const char *delim)
> +{
> +     unsigned int i = 0;
> +     const char *s;
> +
> +     while ((s = z_erofs_list_available_compressors(i)) != NULL) {
> +             if (i++)
> +                     fputs(delim, f);
> +             fputs(s, f);
> +     }
> +     fputc('\n', f);
> +}
> +
>  static void usage(void)
>  {
>       fprintf(stderr, "usage: [options] FILE DIRECTORY\n\n");
> @@ -39,6 +52,8 @@ static void usage(void)
>       fprintf(stderr, " -EX[,...] X=extended options\n");
>       fprintf(stderr, " -T#       set a fixed UNIX timestamp # to all 
> files\n");
>       fprintf(stderr, " --help    display this help and exit\n");
> +     fprintf(stderr, "\nAvailable compressors are: ");
> +     print_available_compressors(stderr, ", ");
>  }
>  
>  static int parse_extended_opts(const char *opts)
> -- 
> 2.17.1
> 

Reply via email to