mailing list of musl libc
 help / color / mirror / code / Atom feed
* getopt() not exposing __optpos - shell needs it
@ 2017-08-28 10:18 Denys Vlasenko
  2017-08-28 15:17 ` Denys Vlasenko
  2017-08-28 15:28 ` Rich Felker
  0 siblings, 2 replies; 9+ messages in thread
From: Denys Vlasenko @ 2017-08-28 10:18 UTC (permalink / raw)
  To: musl, Rich Felker, Waldemar Brodkorb

I am using getopt() in busybox hush shell.
"unset" builtin, for example: it takes -v and -f options.
This works fine.

However, POSIX requires that shells has a "getopts" builtin:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/getopts.html

It is basically an API binding to access getopt() in the shell code:
it uses OPTIND and (in bash) OPTERR on entry, returns a single-char
variable on return and updates OPTIND and OPTARG. Sounds familiar, right?

When I try to do that (use getopt() to implement "getopts"), it hits a snag.
Unlike normal getopt() usage in C programs, where it is called in a loop
with the same argv[] array until parsing is finished,
when it is used from "getopts", each successive call will (usually) have
the same argv[] CONTENTS, but not the ADDRESSES.
(The reason is in how shell works: it re-creates command arguments just before
running a command, since there can be variable substitution, globbing, etc).

Worse yet, it's possible that between invocations of "getopts", there will be
calls to shell builtins which use getopt() intenally. Example:

while getopts "abc" RES -a -bc -abc de; do
    unset -vf func
done

This would not work correctly: getopt() call inside "unset" would modifies
internal libc state which is tracking position in multi-option strings a-la
"-abc". At best, it can skip options. If libc does not check that position is
not beyond strlen(argv[i]), it can return garbage. (With glibc implementation
of getopt(), it would use outright invalid pointers and return garbage
even _without_ "unset" mangling internal state).

The reason for this is that internal state is not fully exposed. It only
shows (and allows modification) of the current argv[i] index, not the
position inside it:

int getopt(int argc, char **argv, const char *optstring);
extern int optind;   // <== internal state

My current "getopts" code preserves optind value across "getopts" calls,
keeping it in $OPTIND as POSIX intends all along:

        cp = get_local_var_value("OPTIND");
        optind = cp ? atoi(cp) : 0;
        optarg = NULL;

        c = getopt(string_array_len(argv), argv, optstring);

        /* Set OPTARG */
        cp = optarg;
        if (cp)
                set_local_var_from_halves("OPTARG", cp);
        else
                unset_local_var("OPTARG");
        /* Convert -1 to "?" */
        exitcode = EXIT_SUCCESS;
        if (c < 0) { /* -1: end of options */
                exitcode = EXIT_FAILURE;
                c = '?';
        }
        /* Set VAR and OPTIND */
        cbuf[0] = c;
        set_local_var_from_halves(var, cbuf);
        set_local_var_from_halves("OPTIND", utoa(optind));

However, position inside argv[OPTIND] can not be preserved across calls:
it is not exposed by libc.

Musl implementation is pretty simple: it has the "int __optpos" variable,
which holds this information.

I propose to export it along with optind et al:

-extern int optind, opterr, optopt;
+extern int optind, __optpos, opterr, optopt;

I know that the general pushback to such ideas is "this is not standard".

Well, the standard is having a defect here: it was designed with only
single-option strings in mind (-a -b, not -ab). Now multi-options
are a must for any meaningful compatibility. Thus, libc must have additional
internal state. Without extending API a bit, it's impossible to use getopt()
for "getopts" (which seems to be what _shell_ standard intends), instead,
shell C code is forced to reimplement getopt().

This makes me think that "optpos" should be added. even if it's "not standard".
This is how standards evolve: real-world use discover deficiencies, APIs
are fixed, then these changes trickle into next revisions of standards.

(Optionally, musl may want to add some robustification here:

        if (!optpos) optpos++;
        if ((k = mbtowc(&c, argv[optind]+optpos, MB_LEN_MAX)) < 0) {

to prevent "argv[optind]+optpos" to point past the end of the string.)
(This can only happen with fairly pathological cases when user changes
argv[] strings mid-parsing, but still).

glibc/uclibc need more extensive changes, since they use a pointer to store
the position. This is a test in busybox tree which fails miserably
because of that:

https://git.busybox.net/busybox/tree/shell/hush_test/hush-getopts/getopt_test_libc_bug.tests
# This test can fail with libc with buggy getopt() implementation.
# If getopt() wants to parse multi-option args (-abc),
# it needs to remember a position within current arg.
#
# If this position is kept as a POINTER, not an offset,
# and if argv[] ADDRESSES (not contents!) change, it blows up.


^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-08-29 17:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-28 10:18 getopt() not exposing __optpos - shell needs it Denys Vlasenko
2017-08-28 15:17 ` Denys Vlasenko
2017-08-28 15:28 ` Rich Felker
2017-08-29 11:32   ` Denys Vlasenko
2017-08-29 12:20     ` Rich Felker
2017-08-29 12:47       ` Denys Vlasenko
2017-08-29 13:07         ` Rich Felker
2017-08-29 16:47           ` Denys Vlasenko
2017-08-29 17:38             ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).