All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] debugfs: restore and tweak original error messaging
@ 2013-12-30  3:47 Eric Whitney
  2013-12-30 21:45 ` Theodore Ts'o
  0 siblings, 1 reply; 2+ messages in thread
From: Eric Whitney @ 2013-12-30  3:47 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso

In response to reviewer comments, commit fe56188b07 included changes
that modified some of the code used to output error messages when
checking user-supplied block numbers.  These changes converted calls
to parse_ulonglong() to calls to strtoblk().  Because strtoblk() calls
parse_ulonglong(), and both output error messages, two redundant and
relatively generic messages were output on each error.

Fix this by removing the error message output from strtoblk(), and
extending it to accept an optional error message argument that it
supplies in lieu of a default to parse_ulonglong().  Also, revert to
the more descriptive original error messages with mods per reviewer
comments, and fix an error message in do_replace_node().

Signed-off-by: Eric Whitney <enwlinux@gmail.com>
---
 debugfs/debugfs.c      | 15 +++++++++------
 debugfs/debugfs.h      |  3 ++-
 debugfs/extent_inode.c | 19 +++++++++----------
 debugfs/icheck.c       |  2 +-
 debugfs/util.c         | 17 ++++++++++-------
 5 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index cf7670b..998af33 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -181,7 +181,8 @@ void do_open_filesys(int argc, char **argv)
 				return;
 			break;
 		case 's':
-			err = strtoblk(argv[0], optarg, &superblock);
+			err = strtoblk(argv[0], optarg,
+				       "superblock block number", &superblock);
 			if (err)
 				return;
 			break;
@@ -284,7 +285,7 @@ void do_init_filesys(int argc, char **argv)
 		return;
 
 	memset(&param, 0, sizeof(struct ext2_super_block));
-	err = strtoblk(argv[0], argv[2], &blocks);
+	err = strtoblk(argv[0], argv[2], "blocks count", &blocks);
 	if (err)
 		return;
 	ext2fs_blocks_count_set(&param, blocks);
@@ -2100,7 +2101,7 @@ void do_bmap(int argc, char *argv[])
 	ino = string_to_inode(argv[1]);
 	if (!ino)
 		return;
-	err = strtoblk(argv[0], argv[2], &blk);
+	err = strtoblk(argv[0], argv[2], "logical block", &blk);
 	if (err)
 		return;
 
@@ -2247,11 +2248,11 @@ void do_punch(int argc, char *argv[])
 	ino = string_to_inode(argv[1]);
 	if (!ino)
 		return;
-	err = strtoblk(argv[0], argv[2], &start);
+	err = strtoblk(argv[0], argv[2], "logical block", &start);
 	if (err)
 		return;
 	if (argc == 4) {
-		err = strtoblk(argv[0], argv[3], &end);
+		err = strtoblk(argv[0], argv[3], "logical block", &end);
 		if (err)
 			return;
 	} else
@@ -2457,7 +2458,9 @@ int main(int argc, char **argv)
 						"block size", 0);
 			break;
 		case 's':
-			retval = strtoblk(argv[0], optarg, &superblock);
+			retval = strtoblk(argv[0], optarg,
+					  "superblock block number",
+					  &superblock);
 			if (retval)
 				return 1;
 			break;
diff --git a/debugfs/debugfs.h b/debugfs/debugfs.h
index 6b4f6ef..33389fa 100644
--- a/debugfs/debugfs.h
+++ b/debugfs/debugfs.h
@@ -39,7 +39,8 @@ extern unsigned long parse_ulong(const char *str, const char *cmd,
 				 const char *descr, int *err);
 extern unsigned long long parse_ulonglong(const char *str, const char *cmd,
 					  const char *descr, int *err);
-extern int strtoblk(const char *cmd, const char *str, blk64_t *ret);
+extern int strtoblk(const char *cmd, const char *str, const char *errmsg,
+		    blk64_t *ret);
 extern int common_args_process(int argc, char *argv[], int min_argc,
 			       int max_argc, const char *cmd,
 			       const char *usage, int flags);
diff --git a/debugfs/extent_inode.c b/debugfs/extent_inode.c
index b3c55f9..8b22f5e 100644
--- a/debugfs/extent_inode.c
+++ b/debugfs/extent_inode.c
@@ -264,15 +264,15 @@ void do_replace_node(int argc, char *argv[])
 		return;
 	}
 
-	err = strtoblk(argv[0], argv[1], &extent.e_lblk);
+	err = strtoblk(argv[0], argv[1], "logical block", &extent.e_lblk);
 	if (err)
 		return;
 
-	extent.e_len = parse_ulong(argv[2], argv[0], "logical block", &err);
+	extent.e_len = parse_ulong(argv[2], argv[0], "length", &err);
 	if (err)
 		return;
 
-	err = strtoblk(argv[0], argv[3], &extent.e_pblk);
+	err = strtoblk(argv[0], argv[3], "physical block", &extent.e_pblk);
 	if (err)
 		return;
 
@@ -338,16 +338,15 @@ void do_insert_node(int argc, char *argv[])
 		return;
 	}
 
-	err = strtoblk(cmd, argv[1], &extent.e_lblk);
+	err = strtoblk(cmd, argv[1], "logical block", &extent.e_lblk);
 	if (err)
 		return;
 
-	extent.e_len = parse_ulong(argv[2], cmd,
-				    "length", &err);
+	extent.e_len = parse_ulong(argv[2], cmd, "length", &err);
 	if (err)
 		return;
 
-	err = strtoblk(cmd, argv[3], &extent.e_pblk);
+	err = strtoblk(cmd, argv[3], "physical block", &extent.e_pblk);
 	if (err)
 		return;
 
@@ -385,11 +384,11 @@ void do_set_bmap(int argc, char **argv)
 		return;
 	}
 
-	err = strtoblk(cmd, argv[1], &logical);
+	err = strtoblk(cmd, argv[1], "logical block", &logical);
 	if (err)
 		return;
 
-	err = strtoblk(cmd, argv[2], &physical);
+	err = strtoblk(cmd, argv[2], "physical block", &physical);
 	if (err)
 		return;
 
@@ -516,7 +515,7 @@ void do_goto_block(int argc, char **argv)
 				       "block [level]", 0))
 		return;
 
-	if (strtoblk(argv[0], argv[1], &blk))
+	if (strtoblk(argv[0], argv[1], NULL, &blk))
 		return;
 
 	if (argc == 3) {
diff --git a/debugfs/icheck.c b/debugfs/icheck.c
index 48f432a..3b9bd14 100644
--- a/debugfs/icheck.c
+++ b/debugfs/icheck.c
@@ -86,7 +86,7 @@ void do_icheck(int argc, char **argv)
 	}
 
 	for (i=1; i < argc; i++) {
-		if (strtoblk(argv[0], argv[i], &bw.barray[i-1].blk))
+		if (strtoblk(argv[0], argv[i], NULL, &bw.barray[i-1].blk))
 			goto error_out;
 	}
 
diff --git a/debugfs/util.c b/debugfs/util.c
index aafbc56..21991cf 100644
--- a/debugfs/util.c
+++ b/debugfs/util.c
@@ -301,17 +301,20 @@ unsigned long long parse_ulonglong(const char *str, const char *cmd,
 
 /*
  * This function will convert a string to a block number.  It returns
- * 0 on success, 1 on failure.
+ * 0 on success, 1 on failure.  On failure, it outputs either an optionally
+ * specified error message or a default.
  */
-int strtoblk(const char *cmd, const char *str, blk64_t *ret)
+int strtoblk(const char *cmd, const char *str, const char *errmsg,
+	     blk64_t *ret)
 {
 	blk64_t	blk;
 	int	err;
 
-	blk = parse_ulonglong(str, cmd, "block number", &err);
+	if (errmsg == NULL)
+		blk = parse_ulonglong(str, cmd, "block number", &err);
+	else
+		blk = parse_ulonglong(str, cmd, errmsg, &err);
 	*ret = blk;
-	if (err)
-		com_err(cmd, 0, "Invalid block number: %s", str);
 	return err;
 }
 
@@ -369,7 +372,7 @@ int common_block_args_process(int argc, char *argv[],
 				"<block> [count]", CHECK_FS_BITMAPS))
 		return 1;
 
-	if (strtoblk(argv[0], argv[1], block))
+	if (strtoblk(argv[0], argv[1], NULL, block))
 		return 1;
 	if (*block == 0) {
 		com_err(argv[0], 0, "Invalid block number 0");
@@ -377,7 +380,7 @@ int common_block_args_process(int argc, char *argv[],
 	}
 
 	if (argc > 2) {
-		err = strtoblk(argv[0], argv[2], count);
+		err = strtoblk(argv[0], argv[2], "count", count);
 		if (err)
 			return 1;
 	}
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: debugfs: restore and tweak original error messaging
  2013-12-30  3:47 [PATCH] debugfs: restore and tweak original error messaging Eric Whitney
@ 2013-12-30 21:45 ` Theodore Ts'o
  0 siblings, 0 replies; 2+ messages in thread
From: Theodore Ts'o @ 2013-12-30 21:45 UTC (permalink / raw)
  To: Eric Whitney; +Cc: linux-ext4

On Sun, Dec 29, 2013 at 10:47:54PM -0500, Eric Whitney wrote:
> In response to reviewer comments, commit fe56188b07 included changes
> that modified some of the code used to output error messages when
> checking user-supplied block numbers.  These changes converted calls
> to parse_ulonglong() to calls to strtoblk().  Because strtoblk() calls
> parse_ulonglong(), and both output error messages, two redundant and
> relatively generic messages were output on each error.
> 
> Fix this by removing the error message output from strtoblk(), and
> extending it to accept an optional error message argument that it
> supplies in lieu of a default to parse_ulonglong().  Also, revert to
> the more descriptive original error messages with mods per reviewer
> comments, and fix an error message in do_replace_node().
> 
> Signed-off-by: Eric Whitney <enwlinux@gmail.com>

Applied to the maint branch, thanks!!

						- Ted

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-12-30 21:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-30  3:47 [PATCH] debugfs: restore and tweak original error messaging Eric Whitney
2013-12-30 21:45 ` Theodore Ts'o

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.