[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK1hOcNk95zZeEaWCX0irwZkAnixYjTsU=GMk8Z9153ZwE9rng@mail.gmail.com>
Date: Mon, 28 Aug 2017 12:18:57 +0200
From: Denys Vlasenko <vda.linux@...glemail.com>
To: musl <musl@...ts.openwall.com>, Rich Felker <dalias@...c.org>,
Waldemar Brodkorb <wbx@...nadk.org>
Subject: getopt() not exposing __optpos - shell needs it
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.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
