[Toybox] Mutually Exclusive Arguments. Bug?

[Rob Landley]

On 06/03/2012 08:07 AM, briggers at ninthfloor.org wrote:
> Hi,
> 
> I was trying to use the ~ control character to implement mutually
> exclusive option arguments.

Yeah, during why week off I came to the conclusion that a lot of the 
args stuff was limited, and specced out new infrastructure for it.  
Unfortunately, I haven't had time to finish implementing it:

--- a/lib/args.c        Sun Jun 03 00:32:12 2012 -0500
+++ b/lib/args.c        Sun Jun 03 21:08:41 2012 -0500
@@ -29,11 +29,12 @@
 //    " " (space char) the "plus an  argument" must be separate
 //        I.E. "-j 3" not "-j3". So "kill -stop" != "kill -s top"
 //
-//     These modify other option letters (previously seen in string):
-//       +X enabling this enables X (switch on)
-//       ~X enabling this disables X (switch off)
-//       !X die with error if X already set (x!x die if x supplied twice)
-//       [yz] needs at least one of y or z. TODO
+//   at the end:
+//     [#xyz] Groups. First char is type:
+//       + enabling one enables all (switch on)
+//       - enabling one disables the rest (switch off)
+//       ! die with error if others already set
+//
 //   at the beginning:
 //     ^ stop at first nonoption argument
 //     <0 die if less than # leftover arguments (default 0)


Commit 583 was part of the cleanup I did adjacent to that:

  http://landley.net/hg/toybox/rev/583

> pwd takes two options -P and -L, which are mutually exclusive. Using the
> ~ control character the optstring should be "L~PP~L". Using this
> optstring gives a "Segmentation fault (core dumped)" error when running
> the pwd command.

There's a debug option that prints error messages if an option string
tries to do something the infrastructure can't do. It defaults to off.

The _new_ way of doing that would be "LP[!LP]"

The problem with the current infrastructure is that you can only refer
to entries it's already seen earlier in the string, so L~P can't find
P because the structure describing P isn't initialized yet, and the
logic to test for a null pointer and print an error is under a config
optino (because it's a "should never happen" in normal use, so wasting
the bytes on an error message makes the code bigger).

It's this bit in lib/args.c by the way:

    if (!*++options && CFG_TOYBOX_DEBUG)
        error_exit("+~! no target");
    // Find this option flag (in previously parsed struct opt)
    for (i=0, opt = new; ; opt = opt->next) {
        if (CFG_TOYBOX_DEBUG && !opt)
            error_exit("+~! unknown target");
        if (opt->c == *options) break;
        i++;
    }

> For optstring "LP~L" the command runs. But since the command line
> arguments are parsed from left to right "-L -P" results in an optflag
> value of 1, and "-P -L" gives an optflag value of 3. Since the options
> are mutually exclusive, the optflag value should be either 1 or 2 (or 0
> incase of no arguments)

Which is exactly why I need to define groups in a different way.

The inability to refer to things which haven't been defined yet is
a design limit in parse_optflaglist(), and the easy way to fix it
is to define a new syntax that goes at the _end_ of the string to
refer to relationships between groups of options. Which is what
I'm doing.

> optstring "L~PP" also gives "Segmentation fault (core dumped)" error.
> 
> It seems disabling an option before it is even defined in the optstring
> is the cause for the segmentation fault. I'm not sure if this is
> intentional or not.

It's a known limit in the design, which I have a plan to address.
I can prioritize finishing this since there's a user waiting, but
my week off just ran out and I go back to work in the morning. :(

> I'm at a loss at trying to implement order independent argument
> exclusion without using ~. Ideas, suggestions?

So was I.  My idea is to rip out the ~ suffix and write better
infrastructure.  (It does mean I can't create non-symmetric
relationships anymore, but I can't think of a use case for that,
and could aways just stick an if() in the command's main if
I need something truly oddball.

For right now, implement it with an if() in main to test for
it, and I'll clean it up to have the option parsing do it
automatically once the infrastructure's in.  Sound good?

Thanks,

Rob
-- 
GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code.
Either it's "mere aggregation", or a license violation.  Pick one.

 1338776366.0