On Sun, Aug 04, 2019 at 01:49:43PM +0530, Pratik Shinde wrote:
> handling the case of incorrect debug level.
> Added an enumerated type for supported debug levels.
> Using 'atoi' in place of 'parse_num_from_str'.
> 
> Signed-off-by: Pratik Shinde <[email protected]>
> ---
>  include/erofs/print.h | 18 +++++++++++++-----
>  mkfs/main.c           | 19 ++++++++-----------
>  2 files changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/include/erofs/print.h b/include/erofs/print.h
> index bc0b8d4..296cbbf 100644
> --- a/include/erofs/print.h
> +++ b/include/erofs/print.h
> @@ -12,6 +12,15 @@
>  #include "config.h"
>  #include <stdio.h>
>  
> +enum {
> +     EROFS_MSG_MIN = 0,
> +     EROFS_ERR     = 0,
> +     EROFS_WARN    = 2,
> +     EROFS_INFO    = 3,
> +     EROFS_DBG     = 7,
> +     EROFS_MSG_MAX = 9
> +};
> +
>  #define FUNC_LINE_FMT "%s() Line[%d] "
>  
>  #ifndef pr_fmt
> @@ -19,7 +28,7 @@
>  #endif
>  
>  #define erofs_dbg(fmt, ...) do {                             \
> -     if (cfg.c_dbg_lvl >= 7) {                               \
> +     if (cfg.c_dbg_lvl >= EROFS_DBG) {                       \
>               fprintf(stdout,                                 \
>                       pr_fmt(fmt),                            \
>                       __func__,                               \
> @@ -29,7 +38,7 @@
>  } while (0)
>  
>  #define erofs_info(fmt, ...) do {                            \
> -     if (cfg.c_dbg_lvl >= 3) {                               \
> +     if (cfg.c_dbg_lvl >= EROFS_INFO) {                      \
>               fprintf(stdout,                                 \
>                       pr_fmt(fmt),                            \
>                       __func__,                               \
> @@ -40,7 +49,7 @@
>  } while (0)
>  
>  #define erofs_warn(fmt, ...) do {                            \
> -     if (cfg.c_dbg_lvl >= 2) {                               \
> +     if (cfg.c_dbg_lvl >= EROFS_WARN) {                      \
>               fprintf(stdout,                                 \
>                       pr_fmt(fmt),                            \
>                       __func__,                               \
> @@ -51,7 +60,7 @@
>  } while (0)
>  
>  #define erofs_err(fmt, ...) do {                             \
> -     if (cfg.c_dbg_lvl >= 0) {                               \
> +     if (cfg.c_dbg_lvl >= EROFS_ERR) {                       \
>               fprintf(stderr,                                 \
>                       "Err: " pr_fmt(fmt),                    \
>                       __func__,                               \
> @@ -64,4 +73,3 @@
>  
>  
>  #endif
> -
> diff --git a/mkfs/main.c b/mkfs/main.c
> index fdb65fd..d915d00 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -30,16 +30,6 @@ static void usage(void)
>       fprintf(stderr, " -EX[,...] X=extended options\n");
>  }
>  
> -u64 parse_num_from_str(const char *str)
> -{
> -     u64 num      = 0;
> -     char *endptr = NULL;
> -
> -     num = strtoull(str, &endptr, 10);
> -     BUG_ON(num == ULLONG_MAX);
> -     return num;
> -}
> -
>  static int parse_extended_opts(const char *opts)
>  {
>  #define MATCH_EXTENTED_OPT(opt, token, keylen) \
> @@ -108,7 +98,14 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
>                       break;
>  
>               case 'd':
> -                     cfg.c_dbg_lvl = parse_num_from_str(optarg);
> +                     cfg.c_dbg_lvl = atoi(optarg);
> +                     if (cfg.c_dbg_lvl < EROFS_MSG_MIN
> +                         || cfg.c_dbg_lvl > EROFS_MSG_MAX) {
> +                             fprintf(stderr,
> +                                     "invalid debug level %d\n",
> +                                     cfg.c_dbg_lvl);

How about using erofs_err as my previous patch attached?
I wonder if there are some specfic reasons to directly use fprintf instead?

I will apply it with this minor fixup (no need to resend again), if you have
other considerations, reply me in this thread, thanks. :)

Thanks,
Gao Xiang

> +                             return -EINVAL;
> +                     }
>                       break;
>  
>               case 'E':
> -- 
> 2.9.3
> 

Reply via email to