[PATCH] btrfs-progs: fix btrfstune silence on failure

[PATCH] btrfs-progs: fix btrfstune silence on failure

[Gui Hecheng]

Originally, btrfstune will fail without any options and just exit
with no failure prompt.
Now, the number of arguments are checked before parse options
and error msg will show up upon failure.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 btrfstune.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/btrfstune.c b/btrfstune.c
index 50724ba..ecaf13d 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -115,6 +115,11 @@ int main(int argc, char *argv[])
 	int skinny_flag = 0;
 	int ret;
 
+	if (argc < 3) {
+		print_usage();
+		return 1;
+	}
+
 	while(1) {
 		int c = getopt(argc, argv, "S:rx");
 		if (c < 0)
@@ -176,6 +181,7 @@ int main(int argc, char *argv[])
 	} else {
 		root->fs_info->readonly = 1;
 		ret = 1;
+		fprintf(stderr, "btrfstune failed\n");
 	}
 	close_ctree(root);
 
-- 
1.8.0.1

Re: [PATCH] btrfs-progs: fix btrfstune silence on failure

[David Sterba]

On Fri, Dec 13, 2013 at 05:59:46PM +0800, Gui Hecheng wrote:
> Originally, btrfstune will fail without any options and just exit
> with no failure prompt.

Works for me:

$ ./btrfstune
usage: btrfstune [options] device
	-S value        enable/disable seeding
	-r              enable extended inode refs
	-x enable skinny metadata extent refs

> Now, the number of arguments are checked before parse options
> and error msg will show up upon failure.

No, the arguments should be parsed first. The btrfstune utility does not
use the same parser helpers like check_argc_exact and actually the bug
you see could be caused by missing optind = 1 before the while () loop.

Can you please test if this helps?

--- a/btrfstune.c
+++ b/btrfstune.c
@@ -115,6 +115,7 @@ int main(int argc, char *argv[])
        int skinny_flag = 0;
        int ret;

+       optind = 1;
        while(1) {
                int c = getopt(argc, argv, "S:rx");
                if (c < 0)

Re: [PATCH] btrfs-progs: fix btrfstune silence on failure

[Wang Shilong]

Hi dave,

> On Fri, Dec 13, 2013 at 05:59:46PM +0800, Gui Hecheng wrote:
>> Originally, btrfstune will fail without any options and just exit
>> with no failure prompt.
> 
> Works for me:
> 
> $ ./btrfstune
> usage: btrfstune [options] device
> 	-S value        enable/disable seeding
> 	-r              enable extended inode refs
> 	-x enable skinny metadata extent refs
This is not the problem that this patch addressed,
you can try this:

# btrfstune /dev/sdb

This will not print out anything though it return 1.

> 
>> Now, the number of arguments are checked before parse options
>> and error msg will show up upon failure.
> 
> No, the arguments should be parsed first. The btrfstune utility does not
> use the same parser helpers like check_argc_exact and actually the bug
> you see could be caused by missing optind = 1 before the while () loop.
> 
> Can you please test if this helps?
> 
> --- a/btrfstune.c
> +++ b/btrfstune.c
> @@ -115,6 +115,7 @@ int main(int argc, char *argv[])
>        int skinny_flag = 0;
>        int ret;
> 
> +       optind = 1;

The default value of optind is 1, though we'd better assign the value.

I think Gui Hecheng s patch is right way to fix the problem, but maybe we can a check after arg passing,
something like:

if (!(seeding_flag + exrefs_flag + skinny_flag))
	fprintf(stderr , "You should assign at least one option for btrfstune");

What is your idea^_^

Thanks,
Wang
>        while(1) {
>                int c = getopt(argc, argv, "S:rx");
>                if (c < 0)
> --
> 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