diff mbox

btrfs-progs: fix btrfstune silence on failure

Message ID 20131217143545.GO6498@twin.jikos.cz (mailing list archive)
State Superseded, archived
Headers show

Commit Message

David Sterba Dec. 17, 2013, 2:35 p.m. UTC
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?

--
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

Comments

Wang Shilong Dec. 17, 2013, 3:03 p.m. UTC | #1
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

--
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
diff mbox

Patch

--- 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)