On 2018年06月18日 20:02, David Sterba wrote: > On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote: >> >> >> On 2018年06月18日 19:34, David Sterba wrote: >>> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote: >>>> I understand that btrfs-progs introduced restrict parameter/option order >>>> to distinguish global and sub-command parameter/option. >>>> >>>> However it's really annoying if one just want to append some new options >>>> to previous command: >>>> >>>> E.g. >>>> # btrfs check /dev/data/btrfs >>>> # !! --check-data-csum >>>> >>>> The last command will fail as current btrfs-progs doesn't allow any >>>> option after parameter. >>>> >>>> >>>> Despite the requirement to distinguish global and subcommand >>>> option/parameter, is there any other requirement for such restrict >>>> option-first-parameter-last policy? >>> >>> I'd say that it's a common and recommended pattern. Getopt is able to >>> reorder the parameters so mixed options and non-options are accepted, >>> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more >>> strict requirement, 'btrfs' option parser works the same regardless of >>> that. >> >> But currently it doesn't work. >> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't >> work. >> It's different from a lot of other common programs. > > With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT > unset, it would work in general, but not for 'btrfs'. > > As this is per-application decision I find it ok, besides that I also > find the 'options anywhere' pattern bad. Does it really save you that > much time while typing the commands with new arguments? There are > movement shortcuts to jump by words, you get the previous command by > up-arrow. About the same number of keystrokes. > > New code needs to be tested, documented and maintained, that's the cost > I find too high for something that's convenience for people who are used > to some shell builtins. >
Well, after some testing, the result looks pretty strange on my side.
With the testing code (*), mostly just copied from check/main.c, it
works to detect the final --check-data-csum without problem.
I'm originally planning to use '-' to make getopt_long() to keep the
original order, but the experiment I did shows it unnecessary.
And furthermore, even changing the optstring of btrfs check
getopt_long() with leading '-', it still doesn't work as expected to
detect non-option parameter.
Is there anything wrong/special in btrfs-progs getopt_long() usage?
Thanks,
Qu
*:
---
#include <stdio.h>
#include <getopt.h>
#include <unistd.h>
int main(int argc, char** argv)
{
char *real_argv[] = { "btrfs check", "device", "--check-data-csum" ,
NULL};
int real_argc = sizeof(real_argv) / sizeof(char *) - 1;
while (1) {
enum { GETOPT_VAL_REPAIR = 257, GETOPT_VAL_INIT_CSUM,
GETOPT_VAL_INIT_EXTENT, GETOPT_VAL_CHECK_CSUM,
GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE,
GETOPT_VAL_MODE, GETOPT_VAL_CLEAR_SPACE_CACHE,
GETOPT_VAL_FORCE };
static const struct option long_options[] = {
{ "super", required_argument, NULL, 's' },
{ "repair", no_argument, NULL, GETOPT_VAL_REPAIR },
{ "readonly", no_argument, NULL, GETOPT_VAL_READONLY },
{ "init-csum-tree", no_argument, NULL,
GETOPT_VAL_INIT_CSUM },
{ "init-extent-tree", no_argument, NULL,
GETOPT_VAL_INIT_EXTENT },
{ "check-data-csum", no_argument, NULL,
GETOPT_VAL_CHECK_CSUM },
{ "backup", no_argument, NULL, 'b' },
{ "subvol-extents", required_argument, NULL, 'E' },
{ "qgroup-report", no_argument, NULL, 'Q' },
{ "tree-root", required_argument, NULL, 'r' },
{ "chunk-root", required_argument, NULL,
GETOPT_VAL_CHUNK_TREE },
{ "progress", no_argument, NULL, 'p' },
{ "mode", required_argument, NULL,
GETOPT_VAL_MODE },
{ "clear-space-cache", required_argument, NULL,
GETOPT_VAL_CLEAR_SPACE_CACHE},
{ "force", no_argument, NULL, GETOPT_VAL_FORCE },
{ NULL, 0, NULL, 0}
};
int c;
c = getopt_long(real_argc, real_argv, "as:br:pEQ",
long_options, NULL);
if (c < 0)
break;
switch (c) {
case 'a':
case 'b':
case 'Q':
case 'E':
case 'p':
printf("option: %c\n", c);
break;
case 's':
case 'r':
printf("option: %c %s\n", c, optarg);
break;
case GETOPT_VAL_CHUNK_TREE:
printf("option: chunk tree %s\n", optarg);
break;
case '?':
case 'h':
printf("option: help\n");
break;
case GETOPT_VAL_REPAIR:
printf("option: repair\n");
break;
case GETOPT_VAL_READONLY:
printf("option: readonly\n");
break;
case GETOPT_VAL_INIT_CSUM:
printf("option: init cum tree\n");
break;
case GETOPT_VAL_INIT_EXTENT:
printf("option: init extent tree\n");
break;
case GETOPT_VAL_CHECK_CSUM:
printf("option: check csum\n");
break;
case GETOPT_VAL_MODE:
printf("option: mode %s\n", optarg);
break;
case GETOPT_VAL_CLEAR_SPACE_CACHE:
printf("option: clear space cache %s\n",
optarg);
break;
case GETOPT_VAL_FORCE:
printf("option: force\n");
break;
default:
printf("unknown option %d\n", c);
break;
}
}
return 0;
}
---
signature.asc
Description: OpenPGP digital signature
