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
Attachment:
signature.asc
Description: OpenPGP digital signature
