[PATCH 1/2] f2fs-tools: correct endianness

[PATCH 1/2] f2fs-tools: correct endianness

[Sheng Yong]

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

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

[Sheng Yong]

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

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

[Jaegeuk Kim]

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

Re: [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 <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

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

[Jaegeuk Kim]

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

Re: [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 <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

Re: [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 <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