* [PATCH 1/2] f2fs-tools: correct endianness
@ 2017-01-19 3:03 Sheng Yong
2017-01-19 3:03 ` [PATCH 2/2] fsck.f2fs: support -p without argument Sheng Yong
0 siblings, 1 reply; 7+ messages in thread
From: Sheng Yong @ 2017-01-19 3:03 UTC (permalink / raw)
To: jaegeuk; +Cc: k, piotr.karbowski, linux-f2fs-devel
It is reported that fsck.f2fs behaves abnormally when running on MIPS32
rel 2 big endian cpu, since incorrect endianness. So let's correct all
endianess issues of f2fs-tools.
Reported-by: <k@vodka.home.kg>
Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
fsck/dir.c | 6 +++---
fsck/dump.c | 10 +++++-----
fsck/f2fs.h | 4 ++--
fsck/mount.c | 12 ++++++------
fsck/node.c | 2 +-
fsck/segment.c | 3 ++-
include/f2fs_fs.h | 8 +++++---
7 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/fsck/dir.c b/fsck/dir.c
index 2009855..d817d27 100644
--- a/fsck/dir.c
+++ b/fsck/dir.c
@@ -134,7 +134,7 @@ static int find_in_level(struct f2fs_sb_info *sbi,struct f2fs_node *dir,
struct dnode_of_data dn = {0};
void *dentry_blk;
int max_slots = 214;
- nid_t ino = dir->footer.ino;
+ nid_t ino = le32_to_cpu(dir->footer.ino);
f2fs_hash_t namehash;
int ret = 0;
@@ -191,7 +191,7 @@ static int f2fs_find_entry(struct f2fs_sb_info *sbi,
return 0;
}
- max_depth = dir->i.i_current_depth;
+ max_depth = le32_to_cpu(dir->i.i_current_depth);
for (level = 0; level < max_depth; level ++) {
if (find_in_level(sbi, dir, level, de))
return 1;
@@ -209,7 +209,7 @@ static void f2fs_update_dentry(nid_t ino, umode_t mode,
int i;
de = &d->dentry[bit_pos];
- de->name_len = len;
+ de->name_len = cpu_to_le16(len);
de->hash_code = name_hash;
memcpy(d->filename[bit_pos], name, len);
d->filename[bit_pos][len] = 0;
diff --git a/fsck/dump.c b/fsck/dump.c
index 8e7c85c..e9d442f 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -81,7 +81,7 @@ void nat_dump(struct f2fs_sb_info *sbi)
"nid:%5u\tino:%5u\toffset:%5u"
"\tblkaddr:%10u\tpack:%d\n",
ni.nid, ni.ino,
- node_block->footer.flag >>
+ le32_to_cpu(node_block->footer.flag) >>
OFFSET_BIT_SHIFT,
ni.blk_addr, pack);
ret = write(fd, buf, strlen(buf));
@@ -100,7 +100,7 @@ void nat_dump(struct f2fs_sb_info *sbi)
"nid:%5u\tino:%5u\toffset:%5u"
"\tblkaddr:%10u\tpack:%d\n",
ni.nid, ni.ino,
- node_block->footer.flag >>
+ le32_to_cpu(node_block->footer.flag) >>
OFFSET_BIT_SHIFT,
ni.blk_addr, pack);
ret = write(fd, buf, strlen(buf));
@@ -335,13 +335,13 @@ static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
for (i = 0; i < 5; i++) {
if (i == 0 || i == 1)
dump_node_blk(sbi, TYPE_DIRECT_NODE,
- node_blk->i.i_nid[i], &ofs);
+ le32_to_cpu(node_blk->i.i_nid[i]), &ofs);
else if (i == 2 || i == 3)
dump_node_blk(sbi, TYPE_INDIRECT_NODE,
- node_blk->i.i_nid[i], &ofs);
+ le32_to_cpu(node_blk->i.i_nid[i]), &ofs);
else if (i == 4)
dump_node_blk(sbi, TYPE_DOUBLE_INDIRECT_NODE,
- node_blk->i.i_nid[i], &ofs);
+ le32_to_cpu(node_blk->i.i_nid[i]), &ofs);
else
ASSERT(0);
}
diff --git a/fsck/f2fs.h b/fsck/f2fs.h
index fbb878a..1ac28eb 100644
--- a/fsck/f2fs.h
+++ b/fsck/f2fs.h
@@ -361,7 +361,7 @@ static inline block_t sum_blk_addr(struct f2fs_sb_info *sbi, int base, int type)
static inline bool IS_VALID_NID(struct f2fs_sb_info *sbi, u32 nid)
{
return (nid <= (NAT_ENTRY_PER_BLOCK *
- F2FS_RAW_SUPER(sbi)->segment_count_nat
+ le32_to_cpu(F2FS_RAW_SUPER(sbi)->segment_count_nat)
<< (sbi->log_blocks_per_seg - 1)));
}
@@ -369,7 +369,7 @@ static inline bool IS_VALID_BLK_ADDR(struct f2fs_sb_info *sbi, u32 addr)
{
int i;
- if (addr >= F2FS_RAW_SUPER(sbi)->block_count ||
+ if (addr >= le64_to_cpu(F2FS_RAW_SUPER(sbi)->block_count) ||
addr < SM_I(sbi)->main_blkaddr) {
DBG(1, "block addr [0x%x]\n", addr);
return 0;
diff --git a/fsck/mount.c b/fsck/mount.c
index 9fcb008..0144e97 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1375,12 +1375,12 @@ void build_sit_area_bitmap(struct f2fs_sb_info *sbi)
ptr += SIT_VBLOCK_MAP_SIZE;
if (se->valid_blocks == 0x0) {
- if (sbi->ckpt->cur_node_segno[0] == segno ||
- sbi->ckpt->cur_data_segno[0] == segno ||
- sbi->ckpt->cur_node_segno[1] == segno ||
- sbi->ckpt->cur_data_segno[1] == segno ||
- sbi->ckpt->cur_node_segno[2] == segno ||
- sbi->ckpt->cur_data_segno[2] == segno) {
+ if (le32_to_cpu(sbi->ckpt->cur_node_segno[0]) == segno ||
+ le32_to_cpu(sbi->ckpt->cur_data_segno[0]) == segno ||
+ le32_to_cpu(sbi->ckpt->cur_node_segno[1]) == segno ||
+ le32_to_cpu(sbi->ckpt->cur_data_segno[1]) == segno ||
+ le32_to_cpu(sbi->ckpt->cur_node_segno[2]) == segno ||
+ le32_to_cpu(sbi->ckpt->cur_data_segno[2]) == segno) {
continue;
} else {
free_segs++;
diff --git a/fsck/node.c b/fsck/node.c
index c2f83b8..fe923e5 100644
--- a/fsck/node.c
+++ b/fsck/node.c
@@ -78,7 +78,7 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
type = CURSEG_COLD_NODE;
if (IS_DNODE(node_blk)) {
- if (S_ISDIR(f2fs_inode->i.i_mode))
+ if (S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)))
type = CURSEG_HOT_NODE;
else
type = CURSEG_WARM_NODE;
diff --git a/fsck/segment.c b/fsck/segment.c
index 9ce8bf5..6b2f6c1 100644
--- a/fsck/segment.c
+++ b/fsck/segment.c
@@ -80,7 +80,8 @@ static void f2fs_write_block(struct f2fs_sb_info *sbi, nid_t ino, void *buffer,
ret = dev_read_block(inode, ni.blk_addr);
ASSERT(ret >= 0);
- if (S_ISDIR(inode->i.i_mode) || S_ISLNK(inode->i.i_mode))
+ if (S_ISDIR(le16_to_cpu(inode->i.i_mode)) ||
+ S_ISLNK(le16_to_cpu(inode->i.i_mode)))
ASSERT(0);
off_in_block = offset & ((1 << F2FS_BLKSIZE_BITS) - 1);
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 78ea939..ef34ca2 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -170,15 +170,17 @@ static inline uint64_t bswap_64(uint64_t val)
#define DISP_u32(ptr, member) \
do { \
assert(sizeof((ptr)->member) <= 4); \
- printf("%-30s" "\t\t[0x%8x : %u]\n", \
- #member, ((ptr)->member), ((ptr)->member)); \
+ printf("%-30s" "\t\t[0x%8x : %u]\n", \
+ #member, le32_to_cpu(((ptr)->member)), \
+ le32_to_cpu(((ptr)->member))); \
} while (0)
#define DISP_u64(ptr, member) \
do { \
assert(sizeof((ptr)->member) == 8); \
printf("%-30s" "\t\t[0x%8llx : %llu]\n", \
- #member, ((ptr)->member), ((ptr)->member)); \
+ #member, le64_to_cpu(((ptr)->member)), \
+ le64_to_cpu(((ptr)->member))); \
} while (0)
#define DISP_utf(ptr, member) \
--
2.10.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] fsck.f2fs: support -p without argument
2017-01-19 3:03 [PATCH 1/2] f2fs-tools: correct endianness Sheng Yong
@ 2017-01-19 3:03 ` Sheng Yong
2017-01-19 23:19 ` Jaegeuk Kim
0 siblings, 1 reply; 7+ messages in thread
From: Sheng Yong @ 2017-01-19 3:03 UTC (permalink / raw)
To: jaegeuk; +Cc: k, piotr.karbowski, linux-f2fs-devel
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.karbowski@gmail.com>
Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
fsck/main.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/fsck/main.c b/fsck/main.c
index 64537cc..78d10fa 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -83,7 +83,7 @@ void f2fs_parse_options(int argc, char *argv[])
char *prog = basename(argv[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,11 +97,15 @@ void f2fs_parse_options(int argc, char *argv[])
* 0: default level, the same as -a
* 1: check meta
*/
- c.preen_mode = atoi(optarg);
- if (c.preen_mode < 0)
+ if (optarg == NULL) {
c.preen_mode = PREEN_MODE_0;
- else if (c.preen_mode >= PREEN_MODE_MAX)
- c.preen_mode = PREEN_MODE_MAX - 1;
+ } else {
+ 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;
+ }
if (c.preen_mode == PREEN_MODE_0)
c.auto_fix = 1;
MSG(0, "Info: Fix the reported corruption in "
--
2.10.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fsck.f2fs: support -p without argument
2017-01-19 3:03 ` [PATCH 2/2] fsck.f2fs: support -p without argument Sheng Yong
@ 2017-01-19 23:19 ` Jaegeuk Kim
2017-01-20 8:23 ` Sheng Yong
0 siblings, 1 reply; 7+ messages in thread
From: Jaegeuk Kim @ 2017-01-19 23:19 UTC (permalink / raw)
To: Sheng Yong; +Cc: k, piotr.karbowski, linux-f2fs-devel
Hi Sheng Yong,
I tested this, but failed on -p with arguments.
Could you take a look at this change?
>From e558ebf31a35619d5625927b50be0c9feef1feac Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@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.karbowski@gmail.com>
Signed-off-by: Sheng Yong <shengyong1@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@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:";
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;
+ }
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);
+ 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;
}
+ 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)
--
2.11.0
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fsck.f2fs: support -p without argument
2017-01-19 23:19 ` Jaegeuk Kim
@ 2017-01-20 8:23 ` Sheng Yong
2017-01-20 9:47 ` Jaegeuk Kim
0 siblings, 1 reply; 7+ messages in thread
From: Sheng Yong @ 2017-01-20 8:23 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: k, piotr.karbowski, linux-f2fs-devel
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 <jaegeuk@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.karbowski@gmail.com>
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fsck.f2fs: support -p without argument
2017-01-20 8:23 ` Sheng Yong
@ 2017-01-20 9:47 ` Jaegeuk Kim
2017-01-20 12:09 ` Sheng Yong
0 siblings, 1 reply; 7+ messages in thread
From: Jaegeuk Kim @ 2017-01-20 9:47 UTC (permalink / raw)
To: Sheng Yong; +Cc: k, piotr.karbowski, linux-f2fs-devel
On 01/20, Sheng Yong wrote:
> 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 <jaegeuk@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.karbowski@gmail.com>
> > Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> > Signed-off-by: Jaegeuk Kim <jaegeuk@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.
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.
> >
> > 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.
That will be handled by default case below?
> > 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]);
> > }
> >
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fsck.f2fs: support -p without argument
2017-01-20 9:47 ` Jaegeuk Kim
@ 2017-01-20 12:09 ` Sheng Yong
2017-01-21 5:54 ` Jaegeuk Kim
0 siblings, 1 reply; 7+ messages in thread
From: Sheng Yong @ 2017-01-20 12:09 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: k, piotr.karbowski, linux-f2fs-devel
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 <jaegeuk@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.karbowski@gmail.com>
Signed-off-by: Sheng Yong <shengyong1@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fsck.f2fs: support -p without argument
2017-01-20 12:09 ` Sheng Yong
@ 2017-01-21 5:54 ` Jaegeuk Kim
0 siblings, 0 replies; 7+ messages in thread
From: Jaegeuk Kim @ 2017-01-21 5:54 UTC (permalink / raw)
To: Sheng Yong; +Cc: k, piotr.karbowski, linux-f2fs-devel
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 <jaegeuk@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.karbowski@gmail.com>
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-21 5:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-19 3:03 [PATCH 1/2] f2fs-tools: correct endianness Sheng Yong
2017-01-19 3:03 ` [PATCH 2/2] fsck.f2fs: support -p without argument Sheng Yong
2017-01-19 23:19 ` Jaegeuk Kim
2017-01-20 8:23 ` Sheng Yong
2017-01-20 9:47 ` Jaegeuk Kim
2017-01-20 12:09 ` Sheng Yong
2017-01-21 5:54 ` Jaegeuk Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.