zsh-workers
 help / color / mirror / code / Atom feed
* [BUG] getopts OPTIND
@ 2018-01-09 16:22 Francisco de Zuviría Allende
  2018-01-09 22:48 ` dana
  0 siblings, 1 reply; 12+ messages in thread
From: Francisco de Zuviría Allende @ 2018-01-09 16:22 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

testfunc() {
    echo $*;
    echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`";
    echo 'I do getopts :a: varname'; getopts ':a:' varname;
    echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`";
    echo 'I do getopts :a: varname'; getopts ':a:' varname;
    echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`";
    echo 'I do getopts :a: varname'; getopts ':a:' varname;
    echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`";
}

(testfunc -a -w -e -r -a)

Execution in bash:

-a -w -e -r -a
OPTIND is 1, next -a
I do getopts :a: varname
OPTIND is 3, next -e
I do getopts :a: varname
OPTIND is 4, next -r
I do getopts :a: varname
OPTIND is 5, next -a

execution in zsh:

-a -w -e -r -a
OPTIND is 1, next -a
I do getopts :a: varname
OPTIND is 3, next -e
I do getopts :a: varname
OPTIND is 3, next -e
I do getopts :a: varname
OPTIND is 4, next -r

Best regards

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2018-01-09 16:22 [BUG] getopts OPTIND Francisco de Zuviría Allende
@ 2018-01-09 22:48 ` dana
  2018-01-09 22:57   ` Bart Schaefer
                     ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: dana @ 2018-01-09 22:48 UTC (permalink / raw)
  To: franciscodezuviria; +Cc: zsh-workers

On 9 Jan 2018, at 10:22, Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
>Execution in bash: ... OPTIND is 5, next -a
>execution in zsh:  ... OPTIND is 4, next -r

I think this fixes it? At least, zsh gives the same output as bash and dash when
i do this. Peter's left an ominous warning about changes to this function that
deserves recognition though...

dana


diff --git a/Src/builtin.c b/Src/builtin.c
index 0211f2721..2550b29f2 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5437,6 +5437,9 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) {
 	p = "?";
     err:
+	/* Keep OPTIND correct if the user doesn't return after the error */
+	optcind = 0;
+	zoptind++;
 	zsfree(zoptarg);
 	setsparam(var, ztrdup(p));
 	if(quiet) {
diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst
index 7eba5a4b1..f6977accb 100644
--- a/Test/B10getopts.ztst
+++ b/Test/B10getopts.ztst
@@ -79,3 +79,20 @@
   test_getopts +x
 1:one illegal option, + variant
 >test_getopts:3: bad option: +x
+
+  t() { local o; repeat $1 { getopts a: o "${@:2}" 2>&1 }; print -r $OPTIND }
+  t 4 -a -w -e -r -a
+  t 5 -a -w -e -a -w -e
+  t 5 -a -w -e -r -ax -a
+0:OPTIND calculation after error (workers/42248)
+*>*: bad option: -e
+*>*: bad option: -r
+*>*: argument expected after -a option
+>6
+*>*: bad option: -e
+*>*: bad option: -e
+>7
+*>*: bad option: -e
+*>*: bad option: -r
+*>*: argument expected after -a option
+>7



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2018-01-09 22:48 ` dana
@ 2018-01-09 22:57   ` Bart Schaefer
  2018-01-09 23:58     ` dana
  2018-01-10  9:05   ` Peter Stephenson
  2021-04-13 23:28   ` dana
  2 siblings, 1 reply; 12+ messages in thread
From: Bart Schaefer @ 2018-01-09 22:57 UTC (permalink / raw)
  To: dana; +Cc: franciscodezuviria, zsh-workers

On Tue, Jan 9, 2018 at 2:48 PM, dana <dana@dana.is> wrote:
>
> I think this fixes it? At least, zsh gives the same output as bash and dash when
> i do this. Peter's left an ominous warning about changes to this function that
> deserves recognition though...

There's a whole thread from 2015 related to this, beginning with workers/35317


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2018-01-09 22:57   ` Bart Schaefer
@ 2018-01-09 23:58     ` dana
  2018-01-10  1:32       ` Francisco de Zuviría Allende
  0 siblings, 1 reply; 12+ messages in thread
From: dana @ 2018-01-09 23:58 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: franciscodezuviria, zsh-workers

On 9 Jan 2018, at 16:57, Bart Schaefer <schaefer@brasslantern.com> wrote:
>There's a whole thread from 2015 related to this, beginning with workers/35317

Oops, sure enough. Sorry.

Well, i'm not sure about POSIX correctness, but for whatever it's worth, i can
confirm that all of the following are in agreement as to how it should behave:

ash (BusyBox)
bash
dash
ksh93
mksh
posh

The ksh that comes with OpenBSD (derived from pdksh i think) seems to stop
updating OPTIND after it encounters an invalid option until it finds either a
legal one or the end of the argument list; after that it's set back to the
correct value.

yash just seems broken somehow, it gives me nonsense.

dana


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2018-01-09 23:58     ` dana
@ 2018-01-10  1:32       ` Francisco de Zuviría Allende
  0 siblings, 0 replies; 12+ messages in thread
From: Francisco de Zuviría Allende @ 2018-01-10  1:32 UTC (permalink / raw)
  To: dana; +Cc: Bart Schaefer, zsh-workers

Such quick, much wow!

On Jan 9, 2018 8:58 PM, "dana" <dana@dana.is> wrote:
>
> On 9 Jan 2018, at 16:57, Bart Schaefer <schaefer@brasslantern.com> wrote:
> >There's a whole thread from 2015 related to this, beginning with workers/35317
>

OK, I just read workers/35317
and also the previous thread http://www.zsh.org/mla/workers//2015/msg00196.html
and also the previous from 2007 http://www.zsh.org/mla/users/2007/msg01191.html

2 key responses from Peter Stephenson:

2007:

>     This is the way you'd normally use it:  you don't know how many options
>    there are, if any, until getopts returns 1.
>
>     However, it's still a bit fishy.  The internal option index handling logic
>     has always been a bit curious; I've lost count of the number of bugs we've
>     had in it.

And also

On Thu, 28 May 2015 18:17:40 +0100
Peter Stephenson <p.stephenson@xxxxxxxxxxx> wrote:

> This is documented behaviour (well, sort of -- the documentation didn't
> say quite what we actually do) so this needs another compatibility fix.
> POSIX_BUILTINS seems appropriate.

(and then submits a cool patch)

I checked man zshbuiltins

       getopts optstring name [ arg ... ]
(...)
              The first option to be examined may be changed by
explicitly assigning to OPTIND.  OPTIND has an initial  value  of
             1,  and  is  normally  set  to  1  upon  entry  to a
shell function and restored upon exit (this is disabled by the
             POSIX_BUILTINS option).

and man zshoptions

       POSIX_BUILTINS <K> <S>
(...)
              Furthermore, the getopts builtin behaves in a
POSIX-compatible fashion in that the associated  variable  OPTIND  is
             not made local to functions.


Now here things get funky. If I try this:

(
    testfunc() {
        echo POSIX_BUILTINS is $options[posixbuiltins]
        echo $*;
        for i in 1 2 3 4; do
            echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo
next $1)`";
            echo 'I do getopts :a: varname'; getopts ':a:' varname;
        done
    }

    (
        setopt POSIX_BUILTINS
        testfunc -a -w -e -r
    )
    (
        unsetopt POSIX_BUILTINS
        testfunc -a -w -e -r
    )
)

I get:

POSIX_BUILTINS is on
-a -w -e -r
OPTIND is 1, next -a
I do getopts :a: varname
OPTIND is 3, next -e
I do getopts :a: varname
OPTIND is 3, next -e
I do getopts :a: varname
OPTIND is 4, next -r
I do getopts :a: varname
POSIX_BUILTINS is off
-a -w -e -r
OPTIND is 1, next -a
I do getopts :a: varname
OPTIND is 3, next -e
I do getopts :a: varname
OPTIND is 3, next -e
I do getopts :a: varname
OPTIND is 4, next -r
I do getopts :a: varname

hmmmmmmm... I's it a bug?

> Well, i'm not sure about POSIX correctness, but for whatever it's worth, i can
> confirm that all of the following are in agreement as to how it should behave:
>
> ash (BusyBox)
> bash
> dash
> ksh93
> mksh
> posh
>

I think there are 2 things to discuss, one is this (possible) bug.
The other is if POSIX_BUILTINS should be on by default or not.
If off by default, New users will get bugs in their code if they
migrate from bash
If on by default, downstream projects will get bugs in their working code

This options does other things as well:

              When this option is set the command builtin can be used
to execute shell builtin commands.   Parameter  assignments
             specified  before  shell  functions  and  special
builtins are kept after the command completes unless the special
             builtin is prefixed with the command builtin.  Special
builtins are ., :, break,  continue,  declare,  eval,  exit,
             export, integer, local, readonly, return, set, shift,
source, times, trap and unset.

             In  addition,  various error conditions associated with
the above builtins or exec cause a non-interactive shell to
             exit and an interactive shell to return to its top-level
processing.

So this is a broather subject and may be better discussed in a new
thread with a suitable title


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2018-01-09 22:48 ` dana
  2018-01-09 22:57   ` Bart Schaefer
@ 2018-01-10  9:05   ` Peter Stephenson
  2021-04-13 23:28   ` dana
  2 siblings, 0 replies; 12+ messages in thread
From: Peter Stephenson @ 2018-01-10  9:05 UTC (permalink / raw)
  To: zsh-workers

On Tue, 9 Jan 2018 16:48:17 -0600
dana <dana@dana.is> wrote:
> On 9 Jan 2018, at 10:22, Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
> >Execution in bash: ... OPTIND is 5, next -a
> >execution in zsh:  ... OPTIND is 4, next -r
> 
> I think this fixes it? At least, zsh gives the same output as bash and dash when
> i do this. Peter's left an ominous warning about changes to this function that
> deserves recognition though...

I've completely lost track of the cases that are real compatibility
problems and which are just broken, because, as just got quoted, the
code is pretty hair-raising anyway.

If we think we can get away with wrapping this change in
isset(POSIXBUILTINS) that will make it easier.  It doesn't sound like it
can be wrong in that case, anyway.

pws


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2018-01-09 22:48 ` dana
  2018-01-09 22:57   ` Bart Schaefer
  2018-01-10  9:05   ` Peter Stephenson
@ 2021-04-13 23:28   ` dana
  2021-04-14 13:04     ` [BUG] getopts OPTIND - yash's behaviour Daniel Shahaf
  2021-04-14 13:08     ` [BUG] getopts OPTIND Daniel Shahaf
  2 siblings, 2 replies; 12+ messages in thread
From: dana @ 2021-04-13 23:28 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: franciscodezuviria, Bart Schaefer

(Resurrecting this per workers/48509)

bin_getopts() has changed a little since i posted my patch before, this looks
a bit weirder. The test is also weird. But i confirmed that this makes it
behave like dash, bash, and mksh (my ksh93 doesn't support local, and i still
don't know what yash is doing):

% pbpaste | zsh-dev -f
<1><1><3><5><7><6>
% pbpaste | zsh-dev -f --posix-builtins
<2><2><3><6><7><7>
% pbpaste | dash
<2><2><3><6><7><7>
% pbpaste | bash
<2><2><3><6><7><7>
% pbpaste | mksh
<2><2><3><6><7><7>
% pbpaste | yash
<3><3><3><7><7><7>

tbh i don't think i fully understand why this needs to work this way, or
whether there are other cases that should be tested. Open to review obv

dana


diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index a7afe42cf..4b9778e40 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -982,7 +982,8 @@ vindex(OPTARG, use of)
 The first option to be examined may be changed by explicitly assigning
 to tt(OPTIND).  tt(OPTIND) has an initial value of tt(1), and is
 normally set to tt(1) upon entry to a shell function and restored
-upon exit (this is disabled by the tt(POSIX_BUILTINS) option).  tt(OPTARG)
+upon exit.  (The tt(POSIX_BUILTINS) option disables this, and also changes
+the way the value is calculated to match other shells).  tt(OPTARG)
 is not reset and retains its value from the most recent call to
 tt(getopts).  If either of tt(OPTIND) or tt(OPTARG) is explicitly
 unset, it remains unset, and the index or option argument is not
diff --git a/Src/builtin.c b/Src/builtin.c
index 26335a2e8..13dfdf8be 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5556,6 +5556,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     /* check for legality */
     if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) {
 	p = "?";
+	/* Keep OPTIND correct if the user doesn't return after the error */
+	if (isset(POSIXBUILTINS)) {
+	    optcind = 0;
+	    zoptind++;
+	}
 	zsfree(zoptarg);
 	setsparam(var, ztrdup(p));
 	if(quiet) {
@@ -5572,6 +5577,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     if(p[1] == ':') {
 	if(optcind == lenstr) {
 	    if(!args[zoptind]) {
+		/* Fix OPTIND as above */
+		if (isset(POSIXBUILTINS)) {
+		    optcind = 0;
+		    zoptind++;
+		}
 		zsfree(zoptarg);
 		if(quiet) {
 		    setsparam(var, ztrdup(":"));
diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst
index 72c9e209e..69b3d63f4 100644
--- a/Test/B10getopts.ztst
+++ b/Test/B10getopts.ztst
@@ -96,3 +96,29 @@
   done
 0:missing option-argument (quiet mode)
 >:,x
+
+  # This function is written so it can be easily referenced against other shells
+  t() {
+    local o i=0 n=$1
+    shift
+    while [ $i -lt $n ]; do
+      i=$(( i + 1 ))
+      getopts a: o "$@" 2> /dev/null
+    done
+    printf '<%d>' "$OPTIND"
+  }
+  # Try all these the native way, then the POSIX_BUILTINS way
+  for 1 in no_posix_builtins posix_builtins; do (
+    setopt $1
+    print -rn - "$1: "
+    t 1 -a
+    t 1 -w
+    t 2 -a -w
+    t 4 -a -w -e -r -a
+    t 5 -a -w -e -a -w -e
+    t 5 -a -w -e -r -ax -a
+    print
+  ); done
+0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248)
+>no_posix_builtins: <1><1><3><5><7><6>
+>posix_builtins: <2><2><3><6><7><7>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND - yash's behaviour
  2021-04-13 23:28   ` dana
@ 2021-04-14 13:04     ` Daniel Shahaf
  2021-04-14 13:08     ` [BUG] getopts OPTIND Daniel Shahaf
  1 sibling, 0 replies; 12+ messages in thread
From: Daniel Shahaf @ 2021-04-14 13:04 UTC (permalink / raw)
  To: dana; +Cc: Zsh hackers list, franciscodezuviria

dana wrote on Tue, Apr 13, 2021 at 18:28:50 -0500:
> +  # This function is written so it can be easily referenced against other shells
> +  t() {
> +    local o i=0 n=$1
> +    shift
> +    while [ $i -lt $n ]; do
> +      i=$(( i + 1 ))
> +      getopts a: o "$@" 2> /dev/null
> +    done
> +    printf '<%d>' "$OPTIND"
> +  }
> +  # Try all these the native way, then the POSIX_BUILTINS way
> +  for 1 in no_posix_builtins posix_builtins; do (
> +    setopt $1
> +    print -rn - "$1: "
> +    <3> t 1 -a

I'm guessing yash notices that -a (at index 1) takes an argument, so it
goes to the top of the loop for the word at index 2, where it increments
OPTIND before noticing that index 2 is actually past the end of the
array.

> +    <3> t 1 -w

I get "1:2" in yash 2.51 interactively.

> +    <3> t 2 -a -w

Consistent with other shells.

> +    <7> t 4 -a -w -e -r -a

- The first word is the -a option.

- The second word is the argument to the -a option.

- The third word is -w, so this call will return an error.  Before
  returning, since the next word to parse is the fourth one, set OPTIND
  to 4.

> +    <7> t 5 -a -w -e -a -w -e

Consistent with other shells.

> +    <7> t 5 -a -w -e -r -ax -a

Consistent with other shells.

> +    print
> +  ); done
> +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248)
> +>no_posix_builtins: <1><1><3><5><7><6>
> +>posix_builtins: <2><2><3><6><7><7>
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2021-04-13 23:28   ` dana
  2021-04-14 13:04     ` [BUG] getopts OPTIND - yash's behaviour Daniel Shahaf
@ 2021-04-14 13:08     ` Daniel Shahaf
  2021-04-18  5:16       ` dana
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel Shahaf @ 2021-04-14 13:08 UTC (permalink / raw)
  To: dana; +Cc: Zsh hackers list, franciscodezuviria

dana wrote on Tue, Apr 13, 2021 at 18:28:50 -0500:
> (Resurrecting this per workers/48509)
> 
> bin_getopts() has changed a little since i posted my patch before, this looks
> a bit weirder. The test is also weird. But i confirmed that this makes it
> behave like dash, bash, and mksh (my ksh93 doesn't support local, and i still
> don't know what yash is doing):
> 
> % pbpaste | zsh-dev -f
> <1><1><3><5><7><6>
> % pbpaste | zsh-dev -f --posix-builtins
> <2><2><3><6><7><7>
> % pbpaste | dash
> <2><2><3><6><7><7>
> % pbpaste | bash
> <2><2><3><6><7><7>
> % pbpaste | mksh
> <2><2><3><6><7><7>
> % pbpaste | yash
> <3><3><3><7><7><7>
> 
> tbh i don't think i fully understand why this needs to work this way, or
> whether there are other cases that should be tested. Open to review obv

Should the descriptions of OPTIND and/or POSIX_BUILTINS in the manual be
extended as well?  (Once the behaviour is decided on, if there are still
open questions about that.)

Some more cases to test:
.
       t 0
       t 1 foo
       t 1 -- foo
       t 1 -b
.
where -b doesn't take an argument.

> @@ -96,3 +96,29 @@
> +  # Try all these the native way, then the POSIX_BUILTINS way
> +  for 1 in no_posix_builtins posix_builtins; do (
> +    setopt $1
> +    print -rn - "$1: "
> +    t 1 -a
> +    t 1 -w

> +    t 2 -a -w
> +    t 4 -a -w -e -r -a
> +    t 5 -a -w -e -a -w -e
> +    t 5 -a -w -e -r -ax -a
> +    print
> +  ); done
> +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248)
> +>no_posix_builtins: <1><1><3><5><7><6>
> +>posix_builtins: <2><2><3><6><7><7>
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2021-04-14 13:08     ` [BUG] getopts OPTIND Daniel Shahaf
@ 2021-04-18  5:16       ` dana
  2021-04-20 21:31         ` Daniel Shahaf
  0 siblings, 1 reply; 12+ messages in thread
From: dana @ 2021-04-18  5:16 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list, franciscodezuviria

On 14 Apr 2021, at 08:08, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Should the descriptions of OPTIND and/or POSIX_BUILTINS in the manual be
> extended as well?

The latter, yes; done that. The former, idk, it doesn't currently mention
anything about how it's calculated or about POSIX_BUILTINS effects so i'm
inclined to leave it alone

On 14 Apr 2021, at 08:08, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Some more cases to test:
> .
>       t 0
>       t 1 foo
>       t 1 -- foo
>       t 1 -b
> .
> where -b doesn't take an argument.

`t 0` doesn't test anything, the loop is just skipped. `t 1 -b` tests the same
thing as `t 1 -w`, but i guess it's confusing that i picked -a -w -e -r as the
options; i've changed them to -a -b -c -d. I've also added the two foo ones
you suggested, as well as just `t 1`. (All three behaved the same as other
shells already)

dana


diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index a7afe42cf..4b9778e40 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -982,7 +982,8 @@ vindex(OPTARG, use of)
 The first option to be examined may be changed by explicitly assigning
 to tt(OPTIND).  tt(OPTIND) has an initial value of tt(1), and is
 normally set to tt(1) upon entry to a shell function and restored
-upon exit (this is disabled by the tt(POSIX_BUILTINS) option).  tt(OPTARG)
+upon exit.  (The tt(POSIX_BUILTINS) option disables this, and also changes
+the way the value is calculated to match other shells).  tt(OPTARG)
 is not reset and retains its value from the most recent call to
 tt(getopts).  If either of tt(OPTIND) or tt(OPTARG) is explicitly
 unset, it remains unset, and the index or option argument is not
diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 714e8a1a1..ffe2d1a0d 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -2239,7 +2239,8 @@ command found in the path.
 
 Furthermore, the tt(getopts) builtin behaves in a POSIX-compatible
 fashion in that the associated variable tt(OPTIND) is not made
-local to functions.
+local to functions, and its value is calculated differently to match
+other shells.
 
 Moreover, the warning and special exit code from
 tt([[ -o )var(non_existent_option)tt( ]]) are suppressed.
diff --git a/Src/builtin.c b/Src/builtin.c
index 26335a2e8..13dfdf8be 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5556,6 +5556,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     /* check for legality */
     if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) {
 	p = "?";
+	/* Keep OPTIND correct if the user doesn't return after the error */
+	if (isset(POSIXBUILTINS)) {
+	    optcind = 0;
+	    zoptind++;
+	}
 	zsfree(zoptarg);
 	setsparam(var, ztrdup(p));
 	if(quiet) {
@@ -5572,6 +5577,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     if(p[1] == ':') {
 	if(optcind == lenstr) {
 	    if(!args[zoptind]) {
+		/* Fix OPTIND as above */
+		if (isset(POSIXBUILTINS)) {
+		    optcind = 0;
+		    zoptind++;
+		}
 		zsfree(zoptarg);
 		if(quiet) {
 		    setsparam(var, ztrdup(":"));
diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst
index 72c9e209e..e50d177c7 100644
--- a/Test/B10getopts.ztst
+++ b/Test/B10getopts.ztst
@@ -96,3 +96,32 @@
   done
 0:missing option-argument (quiet mode)
 >:,x
+
+  # This function is written so it can be easily referenced against other shells
+  t() {
+    local o i=0 n=$1
+    shift
+    while [ $i -lt $n ]; do
+      i=$(( i + 1 ))
+      getopts a: o "$@" 2> /dev/null
+    done
+    printf '<%d>' "$OPTIND"
+  }
+  # Try all these the native way, then the POSIX_BUILTINS way
+  for 1 in no_posix_builtins posix_builtins; do (
+    setopt $1
+    print -rn - "$1: "
+    t 1
+    t 1 foo
+    t 1 -- foo
+    t 1 -a
+    t 1 -b
+    t 2 -a -b
+    t 4 -a -b -c -d -a
+    t 5 -a -b -c -a -b -c
+    t 5 -a -b -c -d -ax -a
+    print
+  ); done
+0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248)
+>no_posix_builtins: <1><1><2><1><1><3><5><7><6>
+>posix_builtins: <1><1><2><2><2><3><6><7><7>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2021-04-18  5:16       ` dana
@ 2021-04-20 21:31         ` Daniel Shahaf
  2021-05-03 23:38           ` dana
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Shahaf @ 2021-04-20 21:31 UTC (permalink / raw)
  To: dana; +Cc: Zsh hackers list, franciscodezuviria

dana wrote on Sun, Apr 18, 2021 at 00:16:52 -0500:
> On 14 Apr 2021, at 08:08, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > Some more cases to test:
> > .
> >       t 0
> >       t 1 foo
> >       t 1 -- foo
> >       t 1 -b
> > .
> > where -b doesn't take an argument.
> 
> `t 0` doesn't test anything, the loop is just skipped.

Yeah, but OPTIND still gets printed, so it does test *something*.

> `t 1 -b` tests the same thing as `t 1 -w`, but i guess it's confusing
> that i picked -a -w -e -r as the options; i've changed them to -a -b
> -c -d. I've also added the two foo ones you suggested, as well as just
> `t 1`. (All three behaved the same as other shells already)

*nod*

> +upon exit.  (The tt(POSIX_BUILTINS) option disables this, and also changes
> +the way the value is calculated to match other shells).  tt(OPTARG)

s/)./.)/

Cheers,

Daniel

> +++ b/Test/B10getopts.ztst
> @@ -96,3 +96,32 @@
>    done
>  0:missing option-argument (quiet mode)
>  >:,x
> +
> +  # This function is written so it can be easily referenced against other shells
> +  t() {
> +    local o i=0 n=$1
> +    shift
> +    while [ $i -lt $n ]; do
> +      i=$(( i + 1 ))
> +      getopts a: o "$@" 2> /dev/null
> +    done
> +    printf '<%d>' "$OPTIND"
> +  }
> +  # Try all these the native way, then the POSIX_BUILTINS way
> +  for 1 in no_posix_builtins posix_builtins; do (
> +    setopt $1
> +    print -rn - "$1: "
> +    t 1
> +    t 1 foo
> +    t 1 -- foo
> +    t 1 -a
> +    t 1 -b
> +    t 2 -a -b
> +    t 4 -a -b -c -d -a
> +    t 5 -a -b -c -a -b -c
> +    t 5 -a -b -c -d -ax -a
> +    print
> +  ); done
> +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248)
> +>no_posix_builtins: <1><1><2><1><1><3><5><7><6>
> +>posix_builtins: <1><1><2><2><2><3><6><7><7>
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [BUG] getopts OPTIND
  2021-04-20 21:31         ` Daniel Shahaf
@ 2021-05-03 23:38           ` dana
  0 siblings, 0 replies; 12+ messages in thread
From: dana @ 2021-05-03 23:38 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list, franciscodezuviria

On 20 Apr 2021, at 16:31, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Yeah, but OPTIND still gets printed, so it does test *something*.

That's true. I'm not sure what guarantees there are about the state of OPTIND
when this test runs, though, so i ended up leaving it out. We can add it later
if you want

Anyway, i've committed this now, with the rest of the feedback + a mention in
README. Sorry i lost track of it before

dana



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-05-03 23:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-09 16:22 [BUG] getopts OPTIND Francisco de Zuviría Allende
2018-01-09 22:48 ` dana
2018-01-09 22:57   ` Bart Schaefer
2018-01-09 23:58     ` dana
2018-01-10  1:32       ` Francisco de Zuviría Allende
2018-01-10  9:05   ` Peter Stephenson
2021-04-13 23:28   ` dana
2021-04-14 13:04     ` [BUG] getopts OPTIND - yash's behaviour Daniel Shahaf
2021-04-14 13:08     ` [BUG] getopts OPTIND Daniel Shahaf
2021-04-18  5:16       ` dana
2021-04-20 21:31         ` Daniel Shahaf
2021-05-03 23:38           ` dana

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

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

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).