[f2fs-dev] [PATCH 2/2] fsck.f2fs: show parse errors neatly

[Jaegeuk Kim]

This patch is just to clean up.

Cc: Sheng Yong 
Signed-off-by: Jaegeuk Kim 
---
 fsck/fsck.h |   8 
 fsck/main.c | 140 
 2 files changed, 93 insertions(+), 55 deletions(-)

diff --git a/fsck/fsck.h b/fsck/fsck.h
index f21e199..5a6f018 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -21,6 +21,14 @@ enum {
PREEN_MODE_MAX
 };
 
+enum {
+   NOERROR,
+   EWRONG_OPT,
+   ENEED_ARG,
+   EUNKNOWN_OPT,
+   EUNKNOWN_ARG,
+};
+
 /* fsck.c */
 struct orphan_info {
u32 nr_inodes;
diff --git a/fsck/main.c b/fsck/main.c
index 0ac1711..6c94a70 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -78,20 +78,6 @@ void sload_usage()
exit(1);
 }
 
-static void __handle_fsck_args(int optopt)
-{
-   switch (optopt) {
-   case 'p':
-   MSG(0, "Info: Use default preen mode\n");
-   c.preen_mode = PREEN_MODE_0;
-   c.auto_fix = 1;
-   break;
-   default:
-   MSG(0, "\tError: Need argument for -%c\n", optopt);
-   fsck_usage();
-   }
-}
-
 static int is_digits(char *optarg)
 {
int i;
@@ -102,24 +88,29 @@ static int is_digits(char *optarg)
return i == strlen(optarg);
 }
 
+static void error_out(void)
+{
+   if (c.func == FSCK)
+   fsck_usage();
+   else if (c.func == DUMP)
+   dump_usage();
+   else if (c.func == DEFRAG)
+   defrag_usage();
+   else if (c.func == RESIZE)
+   resize_usage();
+   else if (c.func == SLOAD)
+   sload_usage();
+}
+
 void f2fs_parse_options(int argc, char *argv[])
 {
int option = 0;
char *prog = basename(argv[0]);
-   int err = 0;
+   int err = NOERROR;
 
if (argc < 2) {
MSG(0, "\tError: Device not specified\n");
-   if (c.func == FSCK)
-   fsck_usage();
-   else if (c.func == DUMP)
-   dump_usage();
-   else if (c.func == DEFRAG)
-   defrag_usage();
-   else if (c.func == RESIZE)
-   resize_usage();
-   else if (c.func == SLOAD)
-   sload_usage();
+   error_out();
}
c.devices[0].path = strdup(argv[argc - 1]);
argv[argc-- - 1] = 0;
@@ -144,9 +135,7 @@ void f2fs_parse_options(int argc, char *argv[])
optind--;
break;
} else if (!is_digits(optarg)) {
-   MSG(0, "\tError: Wrong option -%c %s\n",
-   option, optarg);
-   err = 1;
+   err = EWRONG_OPT;
break;
}
c.preen_mode = atoi(optarg);
@@ -161,14 +150,10 @@ void f2fs_parse_options(int argc, char *argv[])
break;
case 'd':
if (optarg[0] == '-') {
-   MSG(0, "\tError: Need argument for 
-%c\n",
-   option);
-   err = 1;
+   err = ENEED_ARG;
break;
} else if (!is_digits(optarg)) {
-   MSG(0, "\tError: Wrong option -%c %s\n",
-   option, optarg);
-   err = 1;
+   err = EWRONG_OPT;
break;
}
c.dbg_lv = atoi(optarg);
@@ -181,25 +166,27 @@ void f2fs_parse_options(int argc, char *argv[])
case 't':
c.dbg_lv = -1;
break;
+
+
case ':':
-   __handle_fsck_args(optopt);
+   if (optopt == 'p') {
+   MSG(0, "Info: Use default preen 
mode\n");
+   c.preen_mode = PREEN_MODE_0;
+   c.auto_fix = 1;
+   } else {
+   option = optopt;
+   err = ENEED_ARG;
+   break;
+   }
break;
case '?':
-   MSG(0, "\tError: Unknown option %c\n", optopt);
-  

[f2fs-dev] [PATCH 1/2] fsck.f2fs: support -p without argument

[Jaegeuk Kim]

This patch allows fsck run -p without argument. So we could use -p as
-p, -p 0, -p 1. '-p' and '-p 0' have the same meaning as '-a'. '-p 1'
check more meta data than '-a'.

Reported-by: KARBOWSKI Piotr 
Signed-off-by: Sheng Yong 
Signed-off-by: Jaegeuk Kim 
---
 fsck/main.c | 99 +
 1 file changed, 80 insertions(+), 19 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index 39ef8d3..0ac1711 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -17,6 +17,7 @@
  */
 #include "fsck.h"
 #include 
+#include 
 
 struct f2fs_fsck gfsck;
 
@@ -77,13 +78,54 @@ void sload_usage()
exit(1);
 }
 
+static void __handle_fsck_args(int optopt)
+{
+   switch (optopt) {
+   case 'p':
+   MSG(0, "Info: Use default preen mode\n");
+   c.preen_mode = PREEN_MODE_0;
+   c.auto_fix = 1;
+   break;
+   default:
+   MSG(0, "\tError: Need argument for -%c\n", optopt);
+   fsck_usage();
+   }
+}
+
+static int is_digits(char *optarg)
+{
+   int i;
+
+   for (i = 0; i < strlen(optarg); i++)
+   if (!isdigit(optarg[i]))
+   break;
+   return i == strlen(optarg);
+}
+
 void f2fs_parse_options(int argc, char *argv[])
 {
int option = 0;
char *prog = basename(argv[0]);
+   int err = 0;
+
+   if (argc < 2) {
+   MSG(0, "\tError: Device not specified\n");
+   if (c.func == FSCK)
+   fsck_usage();
+   else if (c.func == DUMP)
+   dump_usage();
+   else if (c.func == DEFRAG)
+   defrag_usage();
+   else if (c.func == RESIZE)
+   resize_usage();
+   else if (c.func == SLOAD)
+   sload_usage();
+   }
+   c.devices[0].path = strdup(argv[argc - 1]);
+   argv[argc-- - 1] = 0;
 
if (!strcmp("fsck.f2fs", prog)) {
-   const char *option_string = "ad:fp:t";
+   const char *option_string = ":ad:fp:t";
 
c.func = FSCK;
while ((option = getopt(argc, argv, option_string)) != EOF) {
@@ -97,6 +139,16 @@ void f2fs_parse_options(int argc, char *argv[])
 *  0: default level, the same as -a
 *  1: check meta
 */
+   if (optarg[0] == '-') {
+   c.preen_mode = PREEN_MODE_0;
+   optind--;
+   break;
+   } else if (!is_digits(optarg)) {
+   MSG(0, "\tError: Wrong option -%c %s\n",
+   option, optarg);
+   err = 1;
+   break;
+   }
c.preen_mode = atoi(optarg);
if (c.preen_mode < 0)
c.preen_mode = PREEN_MODE_0;
@@ -108,9 +160,19 @@ void f2fs_parse_options(int argc, char *argv[])
"preen mode %d\n", c.preen_mode);
break;
case 'd':
+   if (optarg[0] == '-') {
+   MSG(0, "\tError: Need argument for 
-%c\n",
+   option);
+   err = 1;
+   break;
+   } else if (!is_digits(optarg)) {
+   MSG(0, "\tError: Wrong option -%c %s\n",
+   option, optarg);
+   err = 1;
+   break;
+   }
c.dbg_lv = atoi(optarg);
-   MSG(0, "Info: Debug level = %d\n",
-   c.dbg_lv);
+   MSG(0, "Info: Debug level = %d\n", c.dbg_lv);
break;
case 'f':
c.fix_on = 1;
@@ -119,11 +181,25 @@ void f2fs_parse_options(int argc, char *argv[])
case 't':
c.dbg_lv = -1;
break;
+   case ':':
+   __handle_fsck_args(optopt);
+   break;
+   case '?':
+   MSG(0, "\tError: Unknown option %c\n", optopt);
+   

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: support -p without argument

[Jaegeuk Kim]

On 01/20, Sheng Yong wrote:
> Hi Jaegeuk,
> 
> On 1/20/2017 5:47 PM, Jaegeuk Kim wrote:
> > On 01/20, Sheng Yong wrote:
> [..]
> >>>  
> >>>   if (!strcmp("fsck.f2fs", prog)) {
> >>> - const char *option_string = "ad:fp:t";
> >>> + const char *option_string = ":a:d:f:p:t:";
> >> I think there is no need to modify options -a/-f/-t, and option_string = 
> >> ":ad:fp:t"
> >> is enough to fix this.
> > 
> > The reason of these messy things was to detect arguments of -a/-f/-t.
> > When I tested this in my ubnutu machine without this, fsck.f2fs would allow
> > -a with an argument like -a 1.
> > 
> [...]
> >> So we could keep these handlings of a/f/t/d as the original ones. And check
> >> if argc > optind to detect if there are unhandled unknown options left at
> >> last.
> > 
> > That will be handled by default case below?
> 
> But the argument of any option cannot be caught by default. I think this maybe
> why you did not get error message when you try -a 1.
> 
> I also test the following modification:

I've resolved the conflict of device_name in f2fs-tools/dev-test, and tested the
below change. It seems it works correctly. :)
Let me merge this first.

Thanks,

> 
> >From e2647f5815718fad57940ae6a1f9536862d99cb1 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim 
> Date: Fri, 20 Jan 2017 17:54:51 +0800
> Subject: [PATCH] fsck.f2fs: support -p without argument
> 
> This patch allows fsck run -p without argument. So we could use -p as
> -p, -p 0, -p 1. '-p' and '-p 0' have the same meaning as '-a'. '-p 1'
> check more meta data than '-a'.
> 
> Reported-by: KARBOWSKI Piotr 
> Signed-off-by: Sheng Yong 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fsck/main.c | 99 
> +
>  1 file changed, 80 insertions(+), 19 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 64537cc..e7282e8 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -17,6 +17,7 @@
>   */
>  #include "fsck.h"
>  #include 
> +#include 
> 
>  struct f2fs_fsck gfsck;
> 
> @@ -77,13 +78,54 @@ void sload_usage()
>   exit(1);
>  }
> 
> +static void __handle_fsck_args(int optopt)
> +{
> + switch (optopt) {
> + case 'p':
> + MSG(0, "Info: Use default preen mode\n");
> + c.preen_mode = PREEN_MODE_0;
> + c.auto_fix = 1;
> + break;
> + default:
> + MSG(0, "\tError: Need argument for -%c\n", optopt);
> + fsck_usage();
> + }
> +}
> +
> +static int is_digits(char *optarg)
> +{
> + int i;
> +
> + for (i = 0; i < strlen(optarg); i++)
> + if (!isdigit(optarg[i]))
> + break;
> + return i == strlen(optarg);
> +}
> +
>  void f2fs_parse_options(int argc, char *argv[])
>  {
>   int option = 0;
>   char *prog = basename(argv[0]);
> + int err = 0;
> +
> + if (argc < 2) {
> + MSG(0, "\tError: Device not specified\n");
> + if (c.func == FSCK)
> + fsck_usage();
> + else if (c.func == DUMP)
> + dump_usage();
> + else if (c.func == DEFRAG)
> + defrag_usage();
> + else if (c.func == RESIZE)
> + resize_usage();
> + else if (c.func == SLOAD)
> + sload_usage();
> + }
> + c.device_name = strdup(argv[argc - 1]);
> + argv[argc-- - 1] = 0;
> 
>   if (!strcmp("fsck.f2fs", prog)) {
> - const char *option_string = "ad:fp:t";
> + const char *option_string = ":ad:fp:t";
> 
>   c.func = FSCK;
>   while ((option = getopt(argc, argv, option_string)) != EOF) {
> @@ -97,6 +139,16 @@ void f2fs_parse_options(int argc, char *argv[])
>*  0: default level, the same as -a
>*  1: check meta
>*/
> + if (optarg[0] == '-') {
> + c.preen_mode = PREEN_MODE_0;
> + optind--;
> + break;
> + } else if (!is_digits(optarg)) {
> + MSG(0, "\tError: Wrong option -%c %s\n",
> + option, optarg);
> + err = 1;
> + break;
> + }
>   c.preen_mode = atoi(optarg);
>   if (c.preen_mode < 0)
>   c.preen_mode = PREEN_MODE_0;
> @@ -108,9 +160,19 @@ void f2fs_parse_options(int argc, char *argv[])
>   "preen mode %d\n", c.preen_mode);
>   break;
>   case 'd':
> + 

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: support -p without argument

[Sheng Yong]

Hi Jaegeuk,

On 1/20/2017 5:47 PM, Jaegeuk Kim wrote:
> On 01/20, Sheng Yong wrote:
[..]
>>>  
>>> if (!strcmp("fsck.f2fs", prog)) {
>>> -   const char *option_string = "ad:fp:t";
>>> +   const char *option_string = ":a:d:f:p:t:";
>> I think there is no need to modify options -a/-f/-t, and option_string = 
>> ":ad:fp:t"
>> is enough to fix this.
> 
> The reason of these messy things was to detect arguments of -a/-f/-t.
> When I tested this in my ubnutu machine without this, fsck.f2fs would allow
> -a with an argument like -a 1.
> 
[...]
>> So we could keep these handlings of a/f/t/d as the original ones. And check
>> if argc > optind to detect if there are unhandled unknown options left at
>> last.
> 
> That will be handled by default case below?

But the argument of any option cannot be caught by default. I think this maybe
why you did not get error message when you try -a 1.

I also test the following modification:

>From e2647f5815718fad57940ae6a1f9536862d99cb1 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim 
Date: Fri, 20 Jan 2017 17:54:51 +0800
Subject: [PATCH] fsck.f2fs: support -p without argument

This patch allows fsck run -p without argument. So we could use -p as
-p, -p 0, -p 1. '-p' and '-p 0' have the same meaning as '-a'. '-p 1'
check more meta data than '-a'.

Reported-by: KARBOWSKI Piotr 
Signed-off-by: Sheng Yong 
Signed-off-by: Jaegeuk Kim 
---
 fsck/main.c | 99 +
 1 file changed, 80 insertions(+), 19 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index 64537cc..e7282e8 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -17,6 +17,7 @@
  */
 #include "fsck.h"
 #include 
+#include 

 struct f2fs_fsck gfsck;

@@ -77,13 +78,54 @@ void sload_usage()
exit(1);
 }

+static void __handle_fsck_args(int optopt)
+{
+   switch (optopt) {
+   case 'p':
+   MSG(0, "Info: Use default preen mode\n");
+   c.preen_mode = PREEN_MODE_0;
+   c.auto_fix = 1;
+   break;
+   default:
+   MSG(0, "\tError: Need argument for -%c\n", optopt);
+   fsck_usage();
+   }
+}
+
+static int is_digits(char *optarg)
+{
+   int i;
+
+   for (i = 0; i < strlen(optarg); i++)
+   if (!isdigit(optarg[i]))
+   break;
+   return i == strlen(optarg);
+}
+
 void f2fs_parse_options(int argc, char *argv[])
 {
int option = 0;
char *prog = basename(argv[0]);
+   int err = 0;
+
+   if (argc < 2) {
+   MSG(0, "\tError: Device not specified\n");
+   if (c.func == FSCK)
+   fsck_usage();
+   else if (c.func == DUMP)
+   dump_usage();
+   else if (c.func == DEFRAG)
+   defrag_usage();
+   else if (c.func == RESIZE)
+   resize_usage();
+   else if (c.func == SLOAD)
+   sload_usage();
+   }
+   c.device_name = strdup(argv[argc - 1]);
+   argv[argc-- - 1] = 0;

if (!strcmp("fsck.f2fs", prog)) {
-   const char *option_string = "ad:fp:t";
+   const char *option_string = ":ad:fp:t";

c.func = FSCK;
while ((option = getopt(argc, argv, option_string)) != EOF) {
@@ -97,6 +139,16 @@ void f2fs_parse_options(int argc, char *argv[])
 *  0: default level, the same as -a
 *  1: check meta
 */
+   if (optarg[0] == '-') {
+   c.preen_mode = PREEN_MODE_0;
+   optind--;
+   break;
+   } else if (!is_digits(optarg)) {
+   MSG(0, "\tError: Wrong option -%c %s\n",
+   option, optarg);
+   err = 1;
+   break;
+   }
c.preen_mode = atoi(optarg);
if (c.preen_mode < 0)
c.preen_mode = PREEN_MODE_0;
@@ -108,9 +160,19 @@ void f2fs_parse_options(int argc, char *argv[])
"preen mode %d\n", c.preen_mode);
break;
case 'd':
+   if (optarg[0] == '-') {
+   MSG(0, "\tError: Need argument for 
-%c\n",
+   option);
+   err = 1;
+   break;
+   } else if 

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: support -p without argument

[Sheng Yong]

Hi Jaegeuk,

On 1/20/2017 7:19 AM, Jaegeuk Kim wrote:
> Hi Sheng Yong,
> 
> I tested this, but failed on -p with arguments.
Sorry, this may because double colon in optsting can only be used
at some Unix-like distributions :(
> 
> Could you take a look at this change?
I tested this patch, it satisifies all option usage.
> 
>>From e558ebf31a35619d5625927b50be0c9feef1feac Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim 
> Date: Thu, 19 Jan 2017 11:03:41 +0800
> Subject: [PATCH] fsck.f2fs: support -p without argument
> 
> This patch allows fsck run -p without argument. So we could use -p as
> -p, -p0, and -p1. '-p' and '-p0' have the same meaning as '-a'.
> '-p1' checks more meta data than '-a'.
> 
> Reported-by: KARBOWSKI Piotr 
> Signed-off-by: Sheng Yong 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fsck/main.c | 129 
> ++--
>  1 file changed, 100 insertions(+), 29 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 39ef8d3..3a58fbb 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -17,6 +17,7 @@
>   */
>  #include "fsck.h"
>  #include 
> +#include 
>  
>  struct f2fs_fsck gfsck;
>  
> @@ -77,53 +78,138 @@ void sload_usage()
>   exit(1);
>  }
>  
> +static void __handle_fsck_args(int optopt)
> +{
> + switch (optopt) {
> + case 'p':
> + printf("Info: Use default preen mode\n");
> + c.preen_mode = PREEN_MODE_0;
> + c.auto_fix = 1;
> + break;
> + case 'a':
> + c.auto_fix = 1;
> + MSG(0, "Info: Fix the reported corruption.\n");
> + break;
> + case 'f':
> + c.fix_on = 1;
> + MSG(0, "Info: Force to fix corruption\n");
> + break;
> + case 't':
> + c.dbg_lv = -1;
> + break;
> + default:
> + printf("\tError: Need argument for -%c\n", optopt);
> + fsck_usage();
> + }
> +}
> +
> +static int is_digits(char *optarg)
> +{
> + int i;
> +
> + for (i = 0; i < strlen(optarg); i++)
> + if (!isdigit(optarg[i]))
> + break;
> + return i == strlen(optarg);
> +}
> +
>  void f2fs_parse_options(int argc, char *argv[])
>  {
>   int option = 0;
>   char *prog = basename(argv[0]);
> + int err = 0;
> +
> + if (argc < 2) {
> + MSG(0, "\tError: Device not specified\n");
> + if (c.func == FSCK)
> + fsck_usage();
> + else if (c.func == DUMP)
> + dump_usage();
> + else if (c.func == DEFRAG)
> + defrag_usage();
> + else if (c.func == RESIZE)
> + resize_usage();
> + else if (c.func == SLOAD)
> + sload_usage();
> + }
> + c.devices[0].path = strdup(argv[argc - 1]);
> + argv[argc-- - 1] = 0;
>  
>   if (!strcmp("fsck.f2fs", prog)) {
> - const char *option_string = "ad:fp:t";
> + const char *option_string = ":a:d:f:p:t:";
I think there is no need to modify options -a/-f/-t, and option_string = 
":ad:fp:t"
is enough to fix this.
>  
>   c.func = FSCK;
>   while ((option = getopt(argc, argv, option_string)) != EOF) {
>   switch (option) {
>   case 'a':
> - c.auto_fix = 1;
> - MSG(0, "Info: Fix the reported corruption.\n");
> + case 'f':
> + case 't':
> + if (optarg) {
> + if (optarg[0] == '-') {
> + optind--;
> + break;
> + }
> + MSG(0, "\tError: Not support arguments"
> + " for -%c\n", option);
> + err = 1;
> + }
So we could keep these handlings of a/f/t/d as the original ones. And check
if argc > optind to detect if there are unhandled unknown options left at
last.
>   break;
>   case 'p':
>   /* preen mode has different levels:
>*  0: default level, the same as -a
>*  1: check meta
>*/
> + if (optarg[0] == '-') {
> + c.preen_mode = PREEN_MODE_0;
> + optind--;
> + break;
> + } else if (!is_digits(optarg)) {
> + MSG(0, "\tError: Wrong option -%c %s\n",
> +