Dear Maintainer, Please have a look to this test: % dash -c 'unset OPTIND' dash: 1: unset: Illegal number: % echo $? 2 % dash -c 'OPTIND=' dash: 1: Illegal number: % echo $? 2 Behaviour with zsh, pdksh, ksh93, bash, busybox ash, yash: $ unset OPTIND $ OPTIND= Found this difference while running a test suite for my script with different POSIX sh's. It looks obvious that OPTIND should not be unset or null so this may not be a bug. What do you think ?
Patch posted to dash@, archived at forwarded-to. наб
I've decided to make getoptsreset always do the reset, regardless of the value of OPTIND. Why else would you assign it anyway?---8<--- Always reset OPTIND if it is modified by the user, regardless of its value. Reported-by: наб <nabijaczleweli@nabijaczleweli.xyz> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/src/options.c b/src/options.c index 4d0a53a..c74e4fe 100644 --- a/src/options.c +++ b/src/options.c @@ -393,7 +393,7 @@ setcmd(int argc, char **argv) void getoptsreset(const char *value) { - shellparam.optind = number(value) ?: 1; + shellparam.optind = 1; shellparam.optoff = -1; }
This interacts terribly with the not fully reliable but nevertheless fairly commonly used "local OPTIND". When the local OPTIND goes out of scope and the prior value of OPTIND is restored, the expectation is not that option processing restarts from the beginning. Cheers, Harald van Dijk
Dash doesn't actually need "local OPTIND". In fact, if you do "local OPTIND" then dash will already be broken because even if OPTIND is reset to the same value, the other internal state optoff will end up being wrong. However, it's not hard to make dash ignore "local OPTIND" and make it work as if it wasn't there.---8<--- Always reset OPTIND if it is modified by the user, regardless of its value. Do not call getoptsreset when returning from a function because of "local OPTIND" as this simply trashes the caller's getopts state. Reported-by: наб <nabijaczleweli@nabijaczleweli.xyz> Reported-by: Harald van Dijk <harald@gigawatt.nl> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/src/options.c b/src/options.c index 4d0a53a..c74e4fe 100644 --- a/src/options.c +++ b/src/options.c @@ -393,7 +393,7 @@ setcmd(int argc, char **argv) void getoptsreset(const char *value) { - shellparam.optind = number(value) ?: 1; + shellparam.optind = 1; shellparam.optoff = -1; } diff --git a/src/var.c b/src/var.c index df432b5..b06b36c 100644 --- a/src/var.c +++ b/src/var.c @@ -93,7 +93,7 @@ struct var varinit[] = { { 0, VSTRFIXED|VTEXTFIXED, "PS1=$ ", 0 }, { 0, VSTRFIXED|VTEXTFIXED, "PS2=> ", 0 }, { 0, VSTRFIXED|VTEXTFIXED, "PS4=+ ", 0 }, - { 0, VSTRFIXED|VTEXTFIXED, defoptindvar, getoptsreset }, + { 0, VSTRFIXED|VTEXTFIXED|VNOFUNC, defoptindvar, getoptsreset }, #ifdef WITH_LINENO { 0, VSTRFIXED|VTEXTFIXED, linenovar, 0 }, #endif @@ -535,7 +535,7 @@ poplocalvars(void) ckfree(vp->text); vp->flags = lvp->flags; vp->text = lvp->text; - if (vp->func) + if (vp->func && !(vp->flags & VNOFUNC)) (*vp->func)(varnull(vp->text)); } ckfree(lvp); -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
I disagree in principle, but I think this is basically fine actually; strictly, we are allowed to do this 98842 If the application sets OPTIND to the value 1, a new set of parameters can be used: either the 98843 current positional parameters or new arg values. Any other attempt to invoke getopts multiple 98844 times in a single shell execution environment with parameters (positional parameters or arg 98845 operands) that are not the same in all invocations, or with an OPTIND value modified to be a 98846 value other than 1, produces unspecified results. but I cannot prove this breaks existing code in the wild. I mean such code most likely definitely exists, but I cannot prove this; but even if it does, it relies on "unspecified" behaviour, which means it "cannot be assured to be portable across conforming implementations" such as dash 1 and dash 2. I still think it should keep working, since it had worked. While I found at least one changelog entry in DCS saying "remove unset OPTIND to work around broken dash shell" by searching for "unset.*OPTIND\b", searching "OPTIND=[^01]" gives me heaps of OPTIND=digit and OPTIND=expression users, but most of them are explicit bash users; the first one with /bin/sh is https://sources.debian.org/src/sptk/3.9-3/debian/scripts/raw2wav/?hl=74#L74 reading OPTIND=0 OPT="" for i in "$@" do OPTIND=`expr $OPTIND + 1` if [ "$OPT" = "" ] then OPTARG="" but this program doesn't use getopts. This turns "unset OPTIND must work but doesn't" + "OPTIND=asd is invalid but probably shouldn't be" + "OPTIND=3 makes getopts parse the third argument (as a happy accident)" into "unset OPTIND works" + "OPTIND=asd works (and restarts getopts as a happy accident)" + "OPTIND=3 restarts getopts (as a happy accident)". So, overall, reasonable, if a tad solomonic.