Hi Gao, I used fprintf here because we are printing this error message in case of invalid 'cfg.c_dbg_lvl'. Hence I thought we cannot rely on erofs_err(). e.g $ mkfs.erofs -d -1 <erofs image> <directory> 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, it will not print anything. 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.
--Pratik. On Sun, Aug 4, 2019 at 3:47 PM Gao Xiang <[email protected]> wrote: > 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 > > >
