btrfs-progs: Avoid interpreting options after "--" when getting unit mode

PATCH Applied Satoru Takeuchi 2 participants 10 messages Mar 14, 2016 – Apr 25, 2016 Btrfs file system

lore

Share & tools

lore

Satoru Takeuchi

author

2016-03-14 00:03 UTC #0 lore raw

PATCH 1/4 btrfs-progs: Avoid interpreting options after "--" when getting unit mode

* actual result

======================================

  # ./btrfs device usage -- -m /btrfs

/dev/sdf1, ID: 1

     Device size:          95367.41MiB
     Data,single:          2056.00MiB
     Metadata,DUP:         2048.00MiB
     System,DUP:             16.00MiB
     Unallocated:          91247.41MiB

======================================

* expected result

======================================

  # ./btrfs device usage -- -m /btrfs

ERROR: can't access '-m': No such file or directory
======================================

Signed-off-by Satoru Takeuchi <[email protected]>

---
This patch can be applied to devel branch (commit: 31266d0ef811).

---
 utils.c | 3 +++
 1 file changed, 3 insertions(+)

utils.c +3 -0

index d7ceaa8..77f0f68 100644
@@ -3024,6 +3024,9 @@ unsigned int get_unit_mode_from_arg(int *argc, char *argv[], int df_mode)
 	int arg_end;
 
 	for (arg_i = 0; arg_i < *argc; arg_i++) {
+		if (!strcmp(argv[arg_i], "--"))
+			break;
+
 		if (!strcmp(argv[arg_i], "--raw")) {
 			unit_mode = UNITS_RAW;
 			argv[arg_i] = NULL;

Signature · 1 line

2.7.0

Satoru Takeuchi

author

2016-03-14 00:13 UTC #1 lore raw

PATCH 2/4 btrfs-progs: fix a reression that "property" with -t option doesn't work

"property" is considered as working without any options from the following commit.

commit 176aeca9a148 ("btrfs-progs: add getopt stubs where needed")

However, we can pass -t option to this command.

* actual result

==================================================

  # ./btrfs prop list -t f /btrfs
  btrfs property list: invalid option -- 't'
  usage: btrfs property list [-t <type>] <object>

Lists available properties with their descriptions for the given object.

Please see the help of 'btrfs property get' for a description of objects and object types.
==================================================

* expected result

==================================================

  # btrfs prop list -t f /btrfs
  label               Set/get label of device
  ==================================================

Signed-off-by Satoru Takeuchi <[email protected]>

---
 cmds-property.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

cmds-property.c +3 -9

index 5b4da26..4c2beb7 100644
@@ -379,9 +379,7 @@ static int cmd_property_get(int argc, char **argv)
 	char *name = NULL;
 	int types = 0;
 
-	clean_args_no_options(argc, argv, cmd_property_get_usage);
-
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 5))
+	if (check_argc_min(argc, 2))
 		usage(cmd_property_get_usage);
 
 	parse_args(argc, argv, cmd_property_get_usage, &types, &object, &name,

@@ -415,9 +413,7 @@ static int cmd_property_set(int argc, char **argv)
 	char *value = NULL;
 	int types = 0;
 
-	clean_args_no_options(argc, argv, cmd_property_set_usage);
-
-	if (check_argc_min(argc, 4) || check_argc_max(argc, 6))
+	if (check_argc_min(argc, 4))
 		usage(cmd_property_set_usage);
 
 	parse_args(argc, argv, cmd_property_set_usage, &types,

@@ -446,9 +442,7 @@ static int cmd_property_list(int argc, char **argv)
 	char *object = NULL;
 	int types = 0;
 
-	clean_args_no_options(argc, argv, cmd_property_list_usage);
-
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
+	if (check_argc_min(argc, 2))
 		usage(cmd_property_list_usage);
 
 	parse_args(argc, argv, cmd_property_list_usage,

Signature · 1 line

2.7.0

David Sterba

2016-03-14 12:23 UTC #2 lore raw

Re: [PATCH 2/4] btrfs-progs: fix a reression that "property" with -t option doesn't work

cmds-property.c static int cmd_property_get(int argc, char **argv) · line 385

Quoted 13 lines · Satoru Takeuchi

cmds-property.c +1 -3

@@ -379,9 +379,7 @@ static int cmd_property_get(int argc, char **argv)
 	char *name = NULL;
 	int types = 0;
 
-	clean_args_no_options(argc, argv, cmd_property_get_usage);
-
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 5))
+	if (check_argc_min(argc, 2))
 		usage(cmd_property_get_usage);
 
 	parse_args(argc, argv, cmd_property_get_usage, &types, &object, &name,

We still need to check the number of non-option arguments here, when the optind is set from parse_args.

Quoted 4 lines

cmds-property.c +1 -1

@@ -415,9 +413,7 @@ static int cmd_property_set(int argc, char **argv)
-	if (check_argc_min(argc, 4) || check_argc_max(argc, 6))
+	if (check_argc_min(argc, 4))
 		usage(cmd_property_set_usage);

...

Quoted 4 lines

cmds-property.c +1 -1

 	parse_args(argc, argv, cmd_property_set_usage, &types,

@@ -446,9 +442,7 @@ static int cmd_property_list(int argc, char **argv)
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
+	if (check_argc_min(argc, 2))

...

Satoru Takeuchi

author

2016-03-15 23:29 UTC #3 lore raw

Re: [PATCH 2/4] btrfs-progs: fix a reression that "property" with -t option doesn't work

Quoted 13 lines · David Sterba

Quoted 12 lines · Satoru Takeuchi

cmds-property.c +1 -3

@@ -379,9 +379,7 @@ static int cmd_property_get(int argc, char **argv)
  	char *name = NULL;
  	int types = 0;
 
-	clean_args_no_options(argc, argv, cmd_property_get_usage);
-
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 5))
+	if (check_argc_min(argc, 2))
  		usage(cmd_property_get_usage);

Quoted 4 lines

cmds-property.c static int cmd_property_get(int argc, char **argv) · line 385
Quoted 1 line
cmds-property.c +0 -0
  	parse_args(argc, argv, cmd_property_get_usage, &types, &object, &name,
We still need to check the number of non-option arguments here, when the optind is set from parse_args.

OK, I'll send a patch which checks the number after getopt.

Thanks,
Satoru

Quoted 15 lines

Quoted 5 lines

cmds-property.c +1 -1

@@ -415,9 +413,7 @@ static int cmd_property_set(int argc, char **argv)
-	if (check_argc_min(argc, 4) || check_argc_max(argc, 6))
+	if (check_argc_min(argc, 4))
  		usage(cmd_property_set_usage);

...

Quoted 4 lines

parse_args(argc, argv, cmd_property_set_usage, &types,

cmds-property.c +1 -1

@@ -446,9 +442,7 @@ static int cmd_property_list(int argc, char **argv)
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
+	if (check_argc_min(argc, 2))

...

Satoru Takeuchi

author

2016-03-14 00:16 UTC #4 lore raw

PATCH 3/4 btrfs-progs: Fix a regression that btrfs filesystem label doesn't work

In reply to Satoru Takeuchi

The number of arguments which is allowed to pass became wrong from the following commit.

commit 176aeca9a148c5e29de0 ("btrfs-progs: add getopt stubs where needed")

* actual result

===========================================================

  # ./btrfs prop get /btrfs label
  label=foo
  # ./btrfs fi label /btrfs
  btrfs filesystem label: too few arguments
  usage: btrfs filesystem label [<device>|<mount_point>] [<newlabel>]

Get or change the label of a filesystem

With one argument, get the label of filesystem on <device>. If <newlabel> is passed, set the filesystem label to <newlabel>.

  # ./btrfs fi label /btrfs bar
  foo
  # ./btrfs prop get /btrfs label
  label=foo
  ===========================================================

* expected result

===========================================================

  # ./btrfs prop get /btrfs label
  label=foo
  # ./btrfs fi label /btrfs
  foo
  # ./btrfs fi label /btrfs bar
  # ./btrfs prop get /btrfs label
  label=bar
  ===========================================================

Signed-off-by Satoru Takeuchi <[email protected]>

---
 cmds-filesystem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

cmds-filesystem.c +3 -3

index 7e74e1b..45c16d2 100644
@@ -1255,11 +1255,11 @@ static int cmd_filesystem_label(int argc, char **argv)
 {
 	clean_args_no_options(argc, argv, cmd_filesystem_label_usage);
 
-	if (check_argc_min(argc - optind, 2) ||
-			check_argc_max(argc - optind, 3))
+	if (check_argc_min(argc - optind, 1) ||
+			check_argc_max(argc - optind, 2))
 		usage(cmd_filesystem_label_usage);
 
-	if (argc - optind > 2) {
+	if (argc - optind > 1) {
 		return set_label(argv[optind], argv[optind + 1]);
 	} else {
 		char label[BTRFS_LABEL_SIZE];

Signature · 1 line

2.7.0

David Sterba

Applied

2016-03-14 12:20 UTC #5 lore raw

Re: [PATCH 3/4] btrfs-progs: Fix a regression that btrfs filesystem label doesn't work

Quoted 37 lines · Satoru Takeuchi

The number of arguments which is allowed to pass became wrong from the following commit.
commit 176aeca9a148c5e29de0 ("btrfs-progs: add getopt stubs where needed")
* actual result
===========================================================
  # ./btrfs prop get /btrfs label
  label=foo
  # ./btrfs fi label /btrfs
  btrfs filesystem label: too few arguments
  usage: btrfs filesystem label [<device>|<mount_point>] [<newlabel>]
Get or change the label of a filesystem
With one argument, get the label of filesystem on <device>. If <newlabel> is passed, set the filesystem label to <newlabel>.
  # ./btrfs fi label /btrfs bar
  foo
  # ./btrfs prop get /btrfs label
  label=foo
  ===========================================================
* expected result
===========================================================
  # ./btrfs prop get /btrfs label
  label=foo
  # ./btrfs fi label /btrfs
  foo
  # ./btrfs fi label /btrfs bar
  # ./btrfs prop get /btrfs label
  label=bar
  ===========================================================

Quoted 1 line

Signed-off-by Satoru Takeuchi <[email protected]>

Applied, thanks.

Satoru Takeuchi

author

2016-03-14 00:28 UTC #6 lore raw

PATCH 4/4 btrfs-progs: "device ready" accepts just one device

In reply to Satoru Takeuchi

* actual result

=======================================

  # ./btrfs device ready /dev/sdb foo
  #
  =======================================

* expecting result

=======================================

  # ./btrfs device ready /dev/sdb foo
  btrfs device ready: too many arguments
  usage: btrfs device ready <device>

Check device to see if it has all of its devices in cache for mounting

#
=======================================

Signed-off-by Satoru Takeuchi <[email protected]>

---
 cmds-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

cmds-device.c +1 -1

index 33da2ce..23656c3 100644
@@ -326,7 +326,7 @@ static int cmd_device_ready(int argc, char **argv)
 
 	clean_args_no_options(argc, argv, cmd_device_ready_usage);
 
-	if (check_argc_min(argc - optind, 1))
+	if (check_argc_exact(argc - optind, 1))
 		usage(cmd_device_ready_usage);
 
 	fd = open("/dev/btrfs-control", O_RDWR);

Signature · 1 line

2.7.0

David Sterba

2016-03-14 12:05 UTC #7 lore raw

Re: [PATCH 4/4] btrfs-progs: "device ready" accepts just one device

Quoted 29 lines · Satoru Takeuchi

* actual result
=======================================
  # ./btrfs device ready /dev/sdb foo
  #
  =======================================
* expecting result
=======================================
  # ./btrfs device ready /dev/sdb foo
  btrfs device ready: too many arguments
  usage: btrfs device ready <device>
Check device to see if it has all of its devices in cache for mounting
#
=======================================
Signed-off-by Satoru Takeuchi <[email protected]>
---
 cmds-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
cmds-device.c +0 -0
index 33da2ce..23656c3 100644

cmds-device.c static int cmd_device_ready(int argc, char **argv) · line 329

Quoted 5 lines

cmds-device.c +1 -1

@@ -326,7 +326,7 @@ static int cmd_device_ready(int argc, char **argv)
 
 	clean_args_no_options(argc, argv, cmd_device_ready_usage);
 
-	if (check_argc_min(argc - optind, 1))
+	if (check_argc_exact(argc - optind, 1))

This silently changes the semantics, so far it accepts multiple values though it contradicts the documentation. I'm not yet sure how to resolve that.

Quoted 9 lines

cmds-device.c +0 -0

 		usage(cmd_device_ready_usage);
 
 	fd = open("/dev/btrfs-control", O_RDWR);

Signature · 5 lines

2.7.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Sterba

2016-04-25 11:59 UTC #8 lore raw

Re: [PATCH 4/4] btrfs-progs: "device ready" accepts just one device

Quoted 31 lines · David Sterba

Quoted 30 lines · Satoru Takeuchi
* actual result
=======================================
  # ./btrfs device ready /dev/sdb foo
  #
  =======================================
* expecting result
=======================================
  # ./btrfs device ready /dev/sdb foo
  btrfs device ready: too many arguments
  usage: btrfs device ready <device>
Check device to see if it has all of its devices in cache for mounting
#
=======================================
Signed-off-by Satoru Takeuchi <[email protected]>
---
 cmds-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
cmds-device.c +0 -0
index 33da2ce..23656c3 100644
@@ -326,7 +326,7 @@ static int cmd_device_ready(int argc, char **argv)
 

Quoted 8 lines

cmds-device.c static int cmd_device_ready(int argc, char **argv) · line 329
Quoted 4 lines
cmds-device.c +1 -1
 	clean_args_no_options(argc, argv, cmd_device_ready_usage);
 
-	if (check_argc_min(argc - optind, 1))
+	if (check_argc_exact(argc - optind, 1))
This silently changes the semantics, so far it accepts multiple values though it contradicts the documentation. I'm not yet sure how to resolve that.

More than one argument did not work before, so I think it's ok to expect just one device. Applied.

David Sterba

Applied

2016-03-14 12:29 UTC #9 lore raw

In reply to Satoru Takeuchi

Quoted 15 lines · Satoru Takeuchi

* actual result
======================================
  # ./btrfs device usage -- -m /btrfs
/dev/sdf1, ID: 1
     Device size:          95367.41MiB
     Data,single:          2056.00MiB
     Metadata,DUP:         2048.00MiB
     System,DUP:             16.00MiB
     Unallocated:          91247.41MiB
======================================
* expected result

Quoted 7 lines

======================================
  # ./btrfs device usage -- -m /btrfs
ERROR: can't access '-m': No such file or directory
======================================
Signed-off-by Satoru Takeuchi <[email protected]>

Applied, thanks.