[PATCH 1/4] btrfs-progs: use usage() to replace the warning msg on no-arg usage

[PATCH 1/4] btrfs-progs: use usage() to replace the warning msg on no-arg usage

[Gui Hecheng]

To be consistent with the other cmds, replace the warning msg
with usage() when send/receive are used without any args.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-receive.c | 6 ++----
 cmds-send.c    | 7 ++-----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/cmds-receive.c b/cmds-receive.c
index cce37a7..72d9cc8 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -951,10 +951,8 @@ int cmd_receive(int argc, char **argv)
 		}
 	}
 
-	if (optind + 1 != argc) {
-		fprintf(stderr, "ERROR: receive needs path to subvolume\n");
-		return 1;
-	}
+	if (optind + 1 != argc)
+		usage(cmd_receive_usage);
 
 	tomnt = argv[optind];
 
diff --git a/cmds-send.c b/cmds-send.c
index fc9a01e..e44770f 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -571,11 +571,8 @@ int cmd_send(int argc, char **argv)
 		}
 	}
 
-	if (optind == argc) {
-		fprintf(stderr, "ERROR: send needs path to snapshot\n");
-		ret = 1;
-		goto out;
-	}
+	if (optind == argc)
+		usage(cmd_send_usage);
 
 	if (outname != NULL) {
 		send.dump_fd = creat(outname, 0600);
-- 
1.8.1.4

[PATCH 2/4] btrfs-progs: remove a dead break before usage()

[Gui Hecheng]

The usage() in help.c calls exit(1), so the break behind is nonsense
and should be removed.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-property.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cmds-property.c b/cmds-property.c
index 3d1f18c..df53f91 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -335,7 +335,6 @@ static void parse_args(int argc, char **argv,
 			break;
 		default:
 			usage(usage_str);
-			break;
 		}
 	}
 
-- 
1.8.1.4

[PATCH 3/4] btrfs-progs: cleanup nonsense ret value assignment

[Gui Hecheng]

The "ret" will be soon used to hold the return value of another function,
assign -1 to it before is nonsense.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 free-space-cache.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/free-space-cache.c b/free-space-cache.c
index 55d7318..bddde24 100644
--- a/free-space-cache.c
+++ b/free-space-cache.c
@@ -291,8 +291,6 @@ static int __load_free_space_cache(struct btrfs_root *root,
 		return 0;
 	}
 
-	ret = -1;
-
 	leaf = path->nodes[0];
 	header = btrfs_item_ptr(leaf, path->slots[0],
 				struct btrfs_free_space_header);
-- 
1.8.1.4

[PATCH 4/4] btrfs-progs: fix fsck leaks on error returns

[Gui Hecheng]

Add close_ctree()s before the "returns" on errors after open_ctree()

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-check.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/cmds-check.c b/cmds-check.c
index 2911af0..81f65a3 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6460,6 +6460,7 @@ int cmd_check(int argc, char **argv)
 	    !extent_buffer_uptodate(info->dev_root->node) ||
 	    !extent_buffer_uptodate(info->chunk_root->node)) {
 		fprintf(stderr, "Critical roots corrupted, unable to fsck the FS\n");
+		close_ctree(info->fs_root);
 		return -EIO;
 	}
 
@@ -6467,11 +6468,14 @@ int cmd_check(int argc, char **argv)
 	if (init_extent_tree) {
 		printf("Creating a new extent tree\n");
 		ret = reinit_extent_tree(info);
-		if (ret)
+		if (ret) {
+			close_ctree(root);
 			return ret;
+		}
 	}
 	if (!extent_buffer_uptodate(info->extent_root->node)) {
 		fprintf(stderr, "Critical roots corrupted, unable to fsck the FS\n");
+		close_ctree(root);
 		return -EIO;
 	}
 
@@ -6483,12 +6487,14 @@ int cmd_check(int argc, char **argv)
 		trans = btrfs_start_transaction(info->csum_root, 1);
 		if (IS_ERR(trans)) {
 			fprintf(stderr, "Error starting transaction\n");
+			close_ctree(root);
 			return PTR_ERR(trans);
 		}
 
 		ret = btrfs_fsck_reinit_root(trans, info->csum_root, 0);
 		if (ret) {
 			fprintf(stderr, "crc root initialization failed\n");
+			close_ctree(root);
 			return -EIO;
 		}
 
-- 
1.8.1.4

Re: [PATCH 4/4] btrfs-progs: fix fsck leaks on error returns

[David Sterba]

On Thu, Feb 13, 2014 at 11:16:38AM +0800, Gui Hecheng wrote:
> @@ -6460,6 +6460,7 @@ int cmd_check(int argc, char **argv)
>  	    !extent_buffer_uptodate(info->dev_root->node) ||
>  	    !extent_buffer_uptodate(info->chunk_root->node)) {
>  		fprintf(stderr, "Critical roots corrupted, unable to fsck the FS\n");
> +		close_ctree(info->fs_root);
>  		return -EIO;

Can you please convert it to the 'goto + single return' pattern?

The other patches are ok, adding them to integration.

Re: [PATCH 4/4] btrfs-progs: fix fsck leaks on error returns

[Gui Hecheng]

On Fri, 2014-02-14 at 18:30 +0100, David Sterba wrote:
> On Thu, Feb 13, 2014 at 11:16:38AM +0800, Gui Hecheng wrote:
> > @@ -6460,6 +6460,7 @@ int cmd_check(int argc, char **argv)
> >  	    !extent_buffer_uptodate(info->dev_root->node) ||
> >  	    !extent_buffer_uptodate(info->chunk_root->node)) {
> >  		fprintf(stderr, "Critical roots corrupted, unable to fsck the FS\n");
> > +		close_ctree(info->fs_root);
> >  		return -EIO;
> 
> Can you please convert it to the 'goto + single return' pattern?

Thanks for your comments, David. I'll rework it soon.

Gui
> The other patches are ok, adding them to integration.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] btrfs-progs: use usage() to replace the warning msg on no-arg usage

[David Sterba]

On Thu, Feb 13, 2014 at 11:16:35AM +0800, Gui Hecheng wrote:
> --- a/cmds-receive.c
> +++ b/cmds-receive.c
> @@ -951,10 +951,8 @@ int cmd_receive(int argc, char **argv)
>  		}
>  	}
>  
> -	if (optind + 1 != argc) {
> -		fprintf(stderr, "ERROR: receive needs path to subvolume\n");
> -		return 1;
> -	}
> +	if (optind + 1 != argc)

FYI, I've replaced this with check_argc_exact

> +		usage(cmd_receive_usage);
>  
>  	tomnt = argv[optind];
>