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 <jaeg...@kernel.org>
> 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 <piotr.karbow...@gmail.com>
> Signed-off-by: Sheng Yong <shengyo...@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org>
> ---
> 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 <libgen.h>
> +#include <ctype.h>
>
> 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",
> + 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.
> + 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 '?'.
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]);
> }
>
> 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
