Package: dash
Version: 0.5.11+git20200708+dd9ef66-5
Severity: normal
Tags: upstream
X-Debbugs-Cc: [email protected]
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 ?
-- System Information:
Debian Release: bullseye/sid
APT prefers unstable
APT policy: (500, 'unstable'), (500, 'testing')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 5.11.0-1.1-liquorix-amd64 (SMP w/16 CPU threads; PREEMPT)
Kernel taint flags: TAINT_OOT_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8),
LANGUAGE=en_US:en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
Versions of packages dash depends on:
ii debconf [debconf-2.0] 1.5.74
ii debianutils 4.11.2
ii dpkg 1.20.7.1
ii libc6 2.31-9
dash recommends no packages.
dash suggests no packages.
Acknowledgement sent
to Herbert Xu <[email protected]>:
Extra info received and forwarded to list. Copy sent to Andrej Shadura <[email protected]>.
(Sun, 19 May 2024 12:06:03 GMT) (full text, mbox, link).
Subject: [PATCH] options: Always reset OPTIND in getoptsreset
Date: Sun, 19 May 2024 19:19:33 +0800
On Wed, Jan 04, 2023 at 04:50:34PM +0000, Harald van Dijk wrote:
>
> Personally, I do think it is better if shells allow assigning arbitrary
> values to OPTIND, including unsetting it, and only have the getopts command
> raise an error if the value is non-numeric, but that is my personal opinion
> and I don't see much of a problem if dash decides to go the other way,
> unless POSIX makes it explicit that it is not permitted for shells to do
> this. FWIW, I did make that change for my version, <https://github.com/hvdijk/gwsh/commit/0df0ba33748eb3881b07cb724fd4fa54422ef2bc>,
> if that change is desired for dash it is easy to apply.
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: наб <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
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;
}
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Acknowledgement sent
to Harald van Dijk <[email protected]>:
Extra info received and forwarded to list. Copy sent to Andrej Shadura <[email protected]>.
(Sun, 19 May 2024 13:33:04 GMT) (full text, mbox, link).
Subject: Re: [PATCH] options: Always reset OPTIND in getoptsreset
Date: Sun, 19 May 2024 14:23:23 +0100
On 19/05/2024 12:19, Herbert Xu wrote:
> On Wed, Jan 04, 2023 at 04:50:34PM +0000, Harald van Dijk wrote:
>>
>> Personally, I do think it is better if shells allow assigning arbitrary
>> values to OPTIND, including unsetting it, and only have the getopts command
>> raise an error if the value is non-numeric, but that is my personal opinion
>> and I don't see much of a problem if dash decides to go the other way,
>> unless POSIX makes it explicit that it is not permitted for shells to do
>> this. FWIW, I did make that change for my version, <https://github.com/hvdijk/gwsh/commit/0df0ba33748eb3881b07cb724fd4fa54422ef2bc>,
>> if that change is desired for dash it is easy to apply.
>
> I've decided to make getoptsreset always do the reset, regardless
> of the value of OPTIND. Why else would you assign it anyway?
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
Acknowledgement sent
to Herbert Xu <[email protected]>:
Extra info received and forwarded to list. Copy sent to Andrej Shadura <[email protected]>.
(Sun, 19 May 2024 14:24:06 GMT) (full text, mbox, link).
Subject: [v2 PATCH] options: Always reset OPTIND in getoptsreset
Date: Sun, 19 May 2024 22:21:42 +0800
On Sun, May 19, 2024 at 02:23:23PM +0100, Harald van Dijk wrote:
>
> 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.
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: наб <[email protected]>
Reported-by: Harald van Dijk <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Sun, May 19, 2024 at 10:21:42PM +0800, Herbert Xu wrote:
> Always reset OPTIND if it is modified by the user, regardless of
> its value.
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.
Debbugs is free software and licensed under the terms of the GNU General
Public License version 2. The current version can be obtained
from https://bugs.debian.org/debbugs-source/.