On 2018年06月19日 12:34, Qu Wenruo wrote: > > > 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?
OK, indeed there is something wrong here.
Just as man page of getopt_long(3) said:
---
NOTES
A program that scans multiple argument vectors, or rescans the
same vector more than once, and wants to make use of GNU exten‐
sions such as '+' and '-' at the start of optstring, or changes
the value of POSIXLY_CORRECT between scans, must reinitialize
getopt() by resetting optind to 0, rather than the traditional
value of 1. (Resetting to 0 forces the invocation of an internal
initialization routine that rechecks POSIXLY_CORRECT and checks
for GNU extensions in optstring.)
---
So, "btrfs check /dev/data/btrfs --check-data-csum" should work.
And the bug is, we reset @optind to 1, other than 0 when calling
getopt_long() on the new argc/argv.
I'll fix it soon.
Thanks,
Qu
>
> Thanks,
> Qu
signature.asc
Description: OpenPGP digital signature
