<div dir="ltr"><div>Hi Gao,</div><div><br></div><div>I used fprintf here because we are printing this error message in case of invalid 'cfg.c_dbg_lvl'. Hence I thought</div><div>we cannot rely on erofs_err().</div><div>e.g</div><div>$ mkfs.erofs -d -1 <erofs image> <directory></div><div>In this case debug level is '-1' which is invalid.If we try to print the error message using erofs_err() with 
c_dbg_lvl = -1,</div><div>it will not print anything.</div><div>While applying the minor fixup, just reset the 
c_dbg_lvl to 0 , so that erofs_err() will be able to log the error message.</div><div><br></div><div>--Pratik.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Aug 4, 2019 at 3:47 PM Gao Xiang <<a href="mailto:hsiangkao@aol.com">hsiangkao@aol.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sun, Aug 04, 2019 at 01:49:43PM +0530, Pratik Shinde wrote:<br>
> handling the case of incorrect debug level.<br>
> Added an enumerated type for supported debug levels.<br>
> Using 'atoi' in place of 'parse_num_from_str'.<br>
> <br>
> Signed-off-by: Pratik Shinde <<a href="mailto:pratikshinde320@gmail.com" target="_blank">pratikshinde320@gmail.com</a>><br>
> ---<br>
>  include/erofs/print.h | 18 +++++++++++++-----<br>
>  mkfs/main.c           | 19 ++++++++-----------<br>
>  2 files changed, 21 insertions(+), 16 deletions(-)<br>
> <br>
> diff --git a/include/erofs/print.h b/include/erofs/print.h<br>
> index bc0b8d4..296cbbf 100644<br>
> --- a/include/erofs/print.h<br>
> +++ b/include/erofs/print.h<br>
> @@ -12,6 +12,15 @@<br>
>  #include "config.h"<br>
>  #include <stdio.h><br>
>  <br>
> +enum {<br>
> +     EROFS_MSG_MIN = 0,<br>
> +     EROFS_ERR     = 0,<br>
> +     EROFS_WARN    = 2,<br>
> +     EROFS_INFO    = 3,<br>
> +     EROFS_DBG     = 7,<br>
> +     EROFS_MSG_MAX = 9<br>
> +};<br>
> +<br>
>  #define FUNC_LINE_FMT "%s() Line[%d] "<br>
>  <br>
>  #ifndef pr_fmt<br>
> @@ -19,7 +28,7 @@<br>
>  #endif<br>
>  <br>
>  #define erofs_dbg(fmt, ...) do {                             \<br>
> -     if (cfg.c_dbg_lvl >= 7) {                               \<br>
> +     if (cfg.c_dbg_lvl >= EROFS_DBG) {                       \<br>
>               fprintf(stdout,                                 \<br>
>                       pr_fmt(fmt),                            \<br>
>                       __func__,                               \<br>
> @@ -29,7 +38,7 @@<br>
>  } while (0)<br>
>  <br>
>  #define erofs_info(fmt, ...) do {                            \<br>
> -     if (cfg.c_dbg_lvl >= 3) {                               \<br>
> +     if (cfg.c_dbg_lvl >= EROFS_INFO) {                      \<br>
>               fprintf(stdout,                                 \<br>
>                       pr_fmt(fmt),                            \<br>
>                       __func__,                               \<br>
> @@ -40,7 +49,7 @@<br>
>  } while (0)<br>
>  <br>
>  #define erofs_warn(fmt, ...) do {                            \<br>
> -     if (cfg.c_dbg_lvl >= 2) {                               \<br>
> +     if (cfg.c_dbg_lvl >= EROFS_WARN) {                      \<br>
>               fprintf(stdout,                                 \<br>
>                       pr_fmt(fmt),                            \<br>
>                       __func__,                               \<br>
> @@ -51,7 +60,7 @@<br>
>  } while (0)<br>
>  <br>
>  #define erofs_err(fmt, ...) do {                             \<br>
> -     if (cfg.c_dbg_lvl >= 0) {                               \<br>
> +     if (cfg.c_dbg_lvl >= EROFS_ERR) {                       \<br>
>               fprintf(stderr,                                 \<br>
>                       "Err: " pr_fmt(fmt),                    \<br>
>                       __func__,                               \<br>
> @@ -64,4 +73,3 @@<br>
>  <br>
>  <br>
>  #endif<br>
> -<br>
> diff --git a/mkfs/main.c b/mkfs/main.c<br>
> index fdb65fd..d915d00 100644<br>
> --- a/mkfs/main.c<br>
> +++ b/mkfs/main.c<br>
> @@ -30,16 +30,6 @@ static void usage(void)<br>
>       fprintf(stderr, " -EX[,...] X=extended options\n");<br>
>  }<br>
>  <br>
> -u64 parse_num_from_str(const char *str)<br>
> -{<br>
> -     u64 num      = 0;<br>
> -     char *endptr = NULL;<br>
> -<br>
> -     num = strtoull(str, &endptr, 10);<br>
> -     BUG_ON(num == ULLONG_MAX);<br>
> -     return num;<br>
> -}<br>
> -<br>
>  static int parse_extended_opts(const char *opts)<br>
>  {<br>
>  #define MATCH_EXTENTED_OPT(opt, token, keylen) \<br>
> @@ -108,7 +98,14 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])<br>
>                       break;<br>
>  <br>
>               case 'd':<br>
> -                     cfg.c_dbg_lvl = parse_num_from_str(optarg);<br>
> +                     cfg.c_dbg_lvl = atoi(optarg);<br>
> +                     if (cfg.c_dbg_lvl < EROFS_MSG_MIN<br>
> +                         || cfg.c_dbg_lvl > EROFS_MSG_MAX) {<br>
> +                             fprintf(stderr,<br>
> +                                     "invalid debug level %d\n",<br>
> +                                     cfg.c_dbg_lvl);<br>
<br>
How about using erofs_err as my previous patch attached?<br>
I wonder if there are some specfic reasons to directly use fprintf instead?<br>
<br>
I will apply it with this minor fixup (no need to resend again), if you have<br>
other considerations, reply me in this thread, thanks. :)<br>
<br>
Thanks,<br>
Gao Xiang<br>
<br>
> +                             return -EINVAL;<br>
> +                     }<br>
>                       break;<br>
>  <br>
>               case 'E':<br>
> -- <br>
> 2.9.3<br>
> <br>
</blockquote></div>