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 <jaeg...@kernel.org>
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 <piotr.karbow...@gmail.com>
Signed-off-by: Sheng Yong <shengyo...@huawei.com>
Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org>
---
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 <libgen.h>
+#include <ctype.h>
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 (!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);
+ err = 1;
+ break;
default:
MSG(0, "\tError: Unknown option %c\n", option);
- fsck_usage();
+ err = 1;
break;
}
+ if (err)
+ fsck_usage();
+ }
+ if (argc > optind) {
+ c.dbg_lv = 0;
+ MSG(0, "\tError: Unknown argument %s\n", argv[optind]);
+ fsck_usage();
}
} else if (!strcmp("dump.f2fs", prog)) {
const char *option_string = "d:i:n:s:a:b:";
@@ -287,21 +363,6 @@ void f2fs_parse_options(int argc, char *argv[])
}
}
}
-
- if ((optind + 1) != argc) {
- 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 = argv[optind];
}
static void do_fsck(struct f2fs_sb_info *sbi)
--
2.10.1
.
>
>>> 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",
>>> + option, optarg);
>>> + err = 1;
>>> + break;
>>> + }
>>> c.preen_mode = atoi(optarg);
>>> if (c.preen_mode < 0)
>>> c.preen_mode = PREEN_MODE_0;
>>> else if (c.preen_mode >= PREEN_MODE_MAX)
>>> - c.preen_mode = PREEN_MODE_MAX - 1;
>>> + c.preen_mode =
>>> + PREEN_MODE_MAX - 1;
>>> if (c.preen_mode == PREEN_MODE_0)
>>> c.auto_fix = 1;
>>> - MSG(0, "Info: Fix the reported corruption in "
>>> - "preen mode %d\n", c.preen_mode);
>>> + MSG(0, "Info: Fix the reported "
>>> + "corruption in preen mode %d\n",
>>> + c.preen_mode);
>>> break;
>>> case 'd':
>>> + if (optarg[0] == '-') {
>>> + printf("\tError: Need argument for
>>> -%c\n",
>>> + option);
>> In this case, we cannot use -1 as the argument of -d. And a trivial fix: use
>> MSG
>> instead of printf would be better.
>
> Yup, it doesn't need to allow -1, and I'll change to use MSG.
>
>>> + 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;
>>> - MSG(0, "Info: Force to fix corruption\n");
>>> + case ':':
>>> + __handle_fsck_args(optopt);
>>> break;
>>> - case 't':
>>> - c.dbg_lv = -1;
>>> + case '?':
>>> + MSG(0, "\tError: Unknown option %c\n", optopt);
>>> + err = 1;
>>> break;
>>> default:
>>> MSG(0, "\tError: Unknown option %c\n", option);
>>> - fsck_usage();
>>> + err = 1;
>>> break;
>>
>> We could integrate '?' and default together, and remove '%c' int the output
>> message, since it is always '?'.
>
> There-in optopt and option can show different failed option strings.
>
>>
>> thanks,
>> Sheng
>>> }
>>> + if (err)
>>> + fsck_usage();
>>> }
>>> } else if (!strcmp("dump.f2fs", prog)) {
>>> const char *option_string = "d:i:n:s:a:b:";
>>> @@ -287,21 +373,6 @@ void f2fs_parse_options(int argc, char *argv[])
>>> }
>>> }
>>> }
>>> -
>>> - if ((optind + 1) != argc) {
>>> - 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[optind]);
BTW, some a patch which modifies c.device_name seems missing here.
thanks,
Sheng
>>> }
>>>
>>> static void do_fsck(struct f2fs_sb_info *sbi)
>>>
>
> .
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
