hush: fix "getopts" builtin to not be upset by other builtins calling getopt()
function old new delta builtin_getopts 363 403 +40 unset_local_var_len 185 215 +30 set_local_var 440 466 +26 reset_traps_to_defaults 151 157 +6 pseudo_exec_argv 320 326 +6 install_special_sighandlers 52 58 +6 pick_sighandler 62 65 +3 execvp_or_die 85 88 +3 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 8/0 up/down: 120/0) Total: 120 bytes Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This commit is contained in:
Denys Vlasenko
parent
0d1eaf407c
commit
238ff98bb8
14
shell/ash_test/ash-getopts/getopt_nested.right
Normal file
14
shell/ash_test/ash-getopts/getopt_nested.right
Normal file
@ -0,0 +1,14 @@
|
||||
var:a
|
||||
var:b
|
||||
var:c
|
||||
var:a
|
||||
var:b
|
||||
var:c
|
||||
Illegal option -d
|
||||
var:?
|
||||
Illegal option -e
|
||||
var:?
|
||||
Illegal option -f
|
||||
var:?
|
||||
var:a
|
||||
End: var:? OPTIND:6
|
||||
21
shell/ash_test/ash-getopts/getopt_nested.tests
Executable file
21
shell/ash_test/ash-getopts/getopt_nested.tests
Executable file
@ -0,0 +1,21 @@
|
||||
# Test that there is no interference of getopt()
|
||||
# in getopts and unset.
|
||||
# It's unclear what "correct" OPTIND values should be
|
||||
# for "b" and "c" results from "-bc": 2? 3?
|
||||
# What we focus on here is that all options are reported
|
||||
# correct number of times and in correct sequence.
|
||||
|
||||
(
|
||||
|
||||
loop=99
|
||||
while getopts "abc" var -a -bc -abc -def -a; do
|
||||
echo "var:$var" #OPTIND:$OPTIND
|
||||
# this may use getopt():
|
||||
unset -ff func
|
||||
test $((--loop)) = 0 && break # BUG if this triggers
|
||||
done
|
||||
echo "End: var:$var OPTIND:$OPTIND"
|
||||
|
||||
) 2>&1 \
|
||||
| sed -e 's/ unrecognized option: / invalid option -- /' \
|
||||
-e 's/ illegal option -- / invalid option -- /' \
|
||||
78
shell/hush.c
78
shell/hush.c
@ -906,6 +906,9 @@ struct globals {
|
||||
#if ENABLE_HUSH_LOOPS
|
||||
unsigned depth_break_continue;
|
||||
unsigned depth_of_loop;
|
||||
#endif
|
||||
#if ENABLE_HUSH_GETOPTS
|
||||
unsigned getopt_count;
|
||||
#endif
|
||||
const char *ifs;
|
||||
const char *cwd;
|
||||
@ -2214,6 +2217,10 @@ static int set_local_var(char *str, unsigned flags)
|
||||
cur->flg_export = 1;
|
||||
if (name_len == 4 && cur->varstr[0] == 'P' && cur->varstr[1] == 'S')
|
||||
cmdedit_update_prompt();
|
||||
#if ENABLE_HUSH_GETOPTS
|
||||
if (strncmp(cur->varstr, "OPTIND=", 7) == 0)
|
||||
G.getopt_count = 0;
|
||||
#endif
|
||||
if (cur->flg_export) {
|
||||
if (flags & SETFLAG_UNEXPORT) {
|
||||
cur->flg_export = 0;
|
||||
@ -2244,6 +2251,10 @@ static int unset_local_var_len(const char *name, int name_len)
|
||||
|
||||
if (!name)
|
||||
return EXIT_SUCCESS;
|
||||
#if ENABLE_HUSH_GETOPTS
|
||||
if (name_len == 6 && strncmp(name, "OPTIND", 6) == 0)
|
||||
G.getopt_count = 0;
|
||||
#endif
|
||||
var_pp = &G.top_var;
|
||||
while ((cur = *var_pp) != NULL) {
|
||||
if (strncmp(cur->varstr, name, name_len) == 0 && cur->varstr[name_len] == '=') {
|
||||
@ -9889,7 +9900,8 @@ Test that VAR is a valid variable name?
|
||||
*/
|
||||
char cbuf[2];
|
||||
const char *cp, *optstring, *var;
|
||||
int c, exitcode;
|
||||
int c, n, exitcode, my_opterr;
|
||||
unsigned count;
|
||||
|
||||
optstring = *++argv;
|
||||
if (!optstring || !(var = *++argv)) {
|
||||
@ -9897,17 +9909,18 @@ Test that VAR is a valid variable name?
|
||||
return EXIT_FAILURE;
|
||||
}
|
||||
|
||||
c = 0;
|
||||
if (argv[1])
|
||||
argv[0] = G.global_argv[0]; /* for error messages in getopt() */
|
||||
else
|
||||
argv = G.global_argv;
|
||||
cbuf[1] = '\0';
|
||||
|
||||
my_opterr = 0;
|
||||
if (optstring[0] != ':') {
|
||||
cp = get_local_var_value("OPTERR");
|
||||
/* 0 if "OPTERR=0", 1 otherwise */
|
||||
c = (!cp || NOT_LONE_CHAR(cp, '0'));
|
||||
my_opterr = (!cp || NOT_LONE_CHAR(cp, '0'));
|
||||
}
|
||||
opterr = c;
|
||||
cp = get_local_var_value("OPTIND");
|
||||
optind = cp ? atoi(cp) : 0;
|
||||
optarg = NULL;
|
||||
cbuf[1] = '\0';
|
||||
|
||||
/* getopts stops on first non-option. Add "+" to force that */
|
||||
/*if (optstring[0] != '+')*/ {
|
||||
@ -9916,11 +9929,47 @@ Test that VAR is a valid variable name?
|
||||
optstring = s;
|
||||
}
|
||||
|
||||
if (argv[1])
|
||||
argv[0] = G.global_argv[0]; /* for error messages */
|
||||
else
|
||||
argv = G.global_argv;
|
||||
c = getopt(string_array_len(argv), argv, optstring);
|
||||
/* Naively, now we should just
|
||||
* cp = get_local_var_value("OPTIND");
|
||||
* optind = cp ? atoi(cp) : 0;
|
||||
* optarg = NULL;
|
||||
* opterr = my_opterr;
|
||||
* c = getopt(string_array_len(argv), argv, optstring);
|
||||
* and be done? Not so fast...
|
||||
* Unlike normal getopt() usage in C programs, here
|
||||
* each successive call will (usually) have the same argv[] CONTENTS,
|
||||
* but not the ADDRESSES. Worse yet, it's possible that between
|
||||
* invocations of "getopts", there will be calls to shell builtins
|
||||
* which use getopt() internally. Example:
|
||||
* while getopts "abc" RES -a -bc -abc de; do
|
||||
* unset -ff func
|
||||
* done
|
||||
* This would not work correctly: getopt() call inside "unset"
|
||||
* modifies internal libc state which is tracking position in
|
||||
* multi-option strings ("-abc"). At best, it can skip options
|
||||
* or return the same option infinitely. With glibc implementation
|
||||
* of getopt(), it would use outright invalid pointers and return
|
||||
* garbage even _without_ "unset" mangling internal state.
|
||||
*
|
||||
* We resort to resetting getopt() state and calling it N times,
|
||||
* until we get Nth result (or failure).
|
||||
* (N == G.getopt_count is reset to 0 whenever OPTIND is [un]set).
|
||||
*/
|
||||
optind = 0; /* reset getopt() state */
|
||||
count = 0;
|
||||
n = string_array_len(argv);
|
||||
do {
|
||||
optarg = NULL;
|
||||
opterr = (count < G.getopt_count) ? 0 : my_opterr;
|
||||
c = getopt(n, argv, optstring);
|
||||
if (c < 0)
|
||||
break;
|
||||
count++;
|
||||
} while (count <= G.getopt_count);
|
||||
|
||||
/* Set OPTIND. Prevent resetting of the magic counter! */
|
||||
set_local_var_from_halves("OPTIND", utoa(optind));
|
||||
G.getopt_count = count; /* "next time, give me N+1'th result" */
|
||||
|
||||
/* Set OPTARG */
|
||||
/* Always set or unset, never left as-is, even on exit/error:
|
||||
@ -9949,10 +9998,9 @@ Test that VAR is a valid variable name?
|
||||
c = '?';
|
||||
}
|
||||
|
||||
/* Set VAR and OPTIND */
|
||||
/* Set VAR */
|
||||
cbuf[0] = c;
|
||||
set_local_var_from_halves(var, cbuf);
|
||||
set_local_var_from_halves("OPTIND", utoa(optind));
|
||||
|
||||
return exitcode;
|
||||
}
|
||||
|
||||
14
shell/hush_test/hush-getopts/getopt_nested.right
Normal file
14
shell/hush_test/hush-getopts/getopt_nested.right
Normal file
@ -0,0 +1,14 @@
|
||||
var:a
|
||||
var:b
|
||||
var:c
|
||||
var:a
|
||||
var:b
|
||||
var:c
|
||||
./getopt_nested.tests: invalid option -- d
|
||||
var:?
|
||||
./getopt_nested.tests: invalid option -- e
|
||||
var:?
|
||||
./getopt_nested.tests: invalid option -- f
|
||||
var:?
|
||||
var:a
|
||||
End: var:? OPTIND:6
|
||||
21
shell/hush_test/hush-getopts/getopt_nested.tests
Executable file
21
shell/hush_test/hush-getopts/getopt_nested.tests
Executable file
@ -0,0 +1,21 @@
|
||||
# Test that there is no interference of getopt()
|
||||
# in getopts and unset.
|
||||
# It's unclear what "correct" OPTIND values should be
|
||||
# for "b" and "c" results from "-bc": 2? 3?
|
||||
# What we focus on here is that all options are reported
|
||||
# correct number of times and in correct sequence.
|
||||
|
||||
(
|
||||
|
||||
loop=99
|
||||
while getopts "abc" var -a -bc -abc -def -a; do
|
||||
echo "var:$var" #OPTIND:$OPTIND
|
||||
# this may use getopt():
|
||||
unset -ff func
|
||||
test $((--loop)) = 0 && break # BUG if this triggers
|
||||
done
|
||||
echo "End: var:$var OPTIND:$OPTIND"
|
||||
|
||||
) 2>&1 \
|
||||
| sed -e 's/ unrecognized option: / invalid option -- /' \
|
||||
-e 's/ illegal option -- / invalid option -- /' \
|
||||
Loading…
Reference in New Issue
Block a user