[PATCH] options: don't error when unsetting OPTIND

[PATCH] options: don't error when unsetting OPTIND

[наб]

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

unset OPTIND ends up calling getoptsreset("") which errors out with
  sh: 1: unset: Illegal number:
or even
  sh: 1: unset: Illegal number: leweli/bin:/usr/l��

Pass the current flags to struct var->func, set the getopts optind to 1
and continue with allowing the unset.

We still forbid OPTIND=, OPTIND=-1, OPTIND=abc, &c.

Fixes: https://bugs.debian.org/985478
---
 src/exec.c    | 2 +-
 src/exec.h    | 2 +-
 src/mail.c    | 2 +-
 src/mail.h    | 2 +-
 src/options.c | 5 ++---
 src/options.h | 2 +-
 src/var.c     | 4 ++--
 src/var.h     | 2 +-
 8 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/exec.c b/src/exec.c
index 87354d4..68fa8ab 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -565,7 +565,7 @@ hashcd(void)
  */
 
 void
-changepath(const char *newval)
+changepath(const char *newval, int unused)
 {
 	const char *new;
 	int idx;
diff --git a/src/exec.h b/src/exec.h
index 423b07e..0f74be4 100644
--- a/src/exec.h
+++ b/src/exec.h
@@ -69,7 +69,7 @@ int hashcmd(int, char **);
 void find_command(char *, struct cmdentry *, int, const char *);
 struct builtincmd *find_builtin(const char *);
 void hashcd(void);
-void changepath(const char *);
+void changepath(const char *, int);
 #ifdef notdef
 void getcmdentry(char *, struct cmdentry *);
 #endif
diff --git a/src/mail.c b/src/mail.c
index 8eacb2d..e81d2b4 100644
--- a/src/mail.c
+++ b/src/mail.c
@@ -109,7 +109,7 @@ chkmail(void)
 
 
 void
-changemail(const char *val)
+changemail(const char *val, int unused)
 {
 	changed++;
 }
diff --git a/src/mail.h b/src/mail.h
index 3c6b21d..70b54a4 100644
--- a/src/mail.h
+++ b/src/mail.h
@@ -35,4 +35,4 @@
  */
 
 void chkmail(void);
-void changemail(const char *);
+void changemail(const char *, int);
diff --git a/src/options.c b/src/options.c
index a46c23b..81f2c4b 100644
--- a/src/options.c
+++ b/src/options.c
@@ -390,10 +390,9 @@ setcmd(int argc, char **argv)
 
 
 void
-getoptsreset(value)
-	const char *value;
+getoptsreset(const char *value, int flags)
 {
-	shellparam.optind = number(value) ?: 1;
+	shellparam.optind = (flags & VUNSET) ? 1 : number(value) ?: 1;
 	shellparam.optoff = -1;
 }
 
diff --git a/src/options.h b/src/options.h
index 975fe33..10bcb88 100644
--- a/src/options.h
+++ b/src/options.h
@@ -83,4 +83,4 @@ int shiftcmd(int, char **);
 int setcmd(int, char **);
 int getoptscmd(int, char **);
 int nextopt(const char *);
-void getoptsreset(const char *);
+void getoptsreset(const char *, int);
diff --git a/src/var.c b/src/var.c
index ef9c2bd..a7d4a92 100644
--- a/src/var.c
+++ b/src/var.c
@@ -266,7 +266,7 @@ struct var *setvareq(char *s, int flags)
 			goto out;
 
 		if (vp->func && (flags & VNOFUNC) == 0)
-			(*vp->func)(strchrnul(s, '=') + 1);
+			(*vp->func)(strchrnul(s, '=') + 1, flags);
 
 		if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 			ckfree(vp->text);
@@ -531,7 +531,7 @@ poplocalvars(void)
 			unsetvar(vp->text);
 		} else {
 			if (vp->func)
-				(*vp->func)(strchrnul(lvp->text, '=') + 1);
+				(*vp->func)(strchrnul(lvp->text, '=') + 1, lvp->flags);
 			if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 				ckfree(vp->text);
 			vp->flags = lvp->flags;
diff --git a/src/var.h b/src/var.h
index aa7575a..4329e22 100644
--- a/src/var.h
+++ b/src/var.h
@@ -56,7 +56,7 @@ struct var {
 	struct var *next;		/* next entry in hash list */
 	int flags;			/* flags are defined above */
 	const char *text;		/* name=value */
-	void (*func)(const char *);
+	void (*func)(const char *, int flags);
 					/* function to be called when  */
 					/* the variable gets set/unset */
 };
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] options: don't error when unsetting OPTIND

[Michael Greenberg]

While we're thinking about it, is this the behavior we want? Or should
`readonly` produce an error?

```
$ getopts abc ARG -ab -c foo
$ echo $OPTIND
2
$ echo $ARG
a
$ readonly OPTIND
$ getopts abc ARG -ab -c foo
/Users/mgree/smoosh/libdash/src/dash: 5: getopts: OPTIND: is read only
$ echo $ARG
a
$ echo $OPTIND
2
```

There are similar questions for PWD and other shell-set nameable
variables:

```
$ readonly PWD
$ pwd
/Users/mgree/pash-medium/budgeting
$ cd ..
/Users/mgree/smoosh/libdash/src/dash: 13: cd: PWD: is read only
$ pwd
/Users/mgree/pash-medium
```

But special parameters (like @ and ?) are outright rejected by
`readonly`.

Bafflingly, the POSIX spec gives the example `readonly HOME PWD`, but
with no explanation about why one might want to do that. (I get making
`HOME` readonly, but `PWD`? Is that supposed to stop `cd` from working?)

Cheers,
Michael

On 2022-12-14 at 03:31:30 AM, наб wrote:

> unset OPTIND ends up calling getoptsreset("") which errors out with
>   sh: 1: unset: Illegal number:
> or even
>   sh: 1: unset: Illegal number: leweli/bin:/usr/l��
>
> Pass the current flags to struct var->func, set the getopts optind to 1
> and continue with allowing the unset.
>
> We still forbid OPTIND=, OPTIND=-1, OPTIND=abc, &c.
>
> Fixes: https://bugs.debian.org/985478
> ---
>  src/exec.c    | 2 +-
>  src/exec.h    | 2 +-
>  src/mail.c    | 2 +-
>  src/mail.h    | 2 +-
>  src/options.c | 5 ++---
>  src/options.h | 2 +-
>  src/var.c     | 4 ++--
>  src/var.h     | 2 +-
>  8 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/src/exec.c b/src/exec.c
> index 87354d4..68fa8ab 100644
> --- a/src/exec.c
> +++ b/src/exec.c
> @@ -565,7 +565,7 @@ hashcd(void)
>   */
>  
>  void
> -changepath(const char *newval)
> +changepath(const char *newval, int unused)
>  {
>  	const char *new;
>  	int idx;
> diff --git a/src/exec.h b/src/exec.h
> index 423b07e..0f74be4 100644
> --- a/src/exec.h
> +++ b/src/exec.h
> @@ -69,7 +69,7 @@ int hashcmd(int, char **);
>  void find_command(char *, struct cmdentry *, int, const char *);
>  struct builtincmd *find_builtin(const char *);
>  void hashcd(void);
> -void changepath(const char *);
> +void changepath(const char *, int);
>  #ifdef notdef
>  void getcmdentry(char *, struct cmdentry *);
>  #endif
> diff --git a/src/mail.c b/src/mail.c
> index 8eacb2d..e81d2b4 100644
> --- a/src/mail.c
> +++ b/src/mail.c
> @@ -109,7 +109,7 @@ chkmail(void)
>  
>  
>  void
> -changemail(const char *val)
> +changemail(const char *val, int unused)
>  {
>  	changed++;
>  }
> diff --git a/src/mail.h b/src/mail.h
> index 3c6b21d..70b54a4 100644
> --- a/src/mail.h
> +++ b/src/mail.h
> @@ -35,4 +35,4 @@
>   */
>  
>  void chkmail(void);
> -void changemail(const char *);
> +void changemail(const char *, int);
> diff --git a/src/options.c b/src/options.c
> index a46c23b..81f2c4b 100644
> --- a/src/options.c
> +++ b/src/options.c
> @@ -390,10 +390,9 @@ setcmd(int argc, char **argv)
>  
>  
>  void
> -getoptsreset(value)
> -	const char *value;
> +getoptsreset(const char *value, int flags)
>  {
> -	shellparam.optind = number(value) ?: 1;
> +	shellparam.optind = (flags & VUNSET) ? 1 : number(value) ?: 1;
>  	shellparam.optoff = -1;
>  }
>  
> diff --git a/src/options.h b/src/options.h
> index 975fe33..10bcb88 100644
> --- a/src/options.h
> +++ b/src/options.h
> @@ -83,4 +83,4 @@ int shiftcmd(int, char **);
>  int setcmd(int, char **);
>  int getoptscmd(int, char **);
>  int nextopt(const char *);
> -void getoptsreset(const char *);
> +void getoptsreset(const char *, int);
> diff --git a/src/var.c b/src/var.c
> index ef9c2bd..a7d4a92 100644
> --- a/src/var.c
> +++ b/src/var.c
> @@ -266,7 +266,7 @@ struct var *setvareq(char *s, int flags)
>  			goto out;
>  
>  		if (vp->func && (flags & VNOFUNC) == 0)
> -			(*vp->func)(strchrnul(s, '=') + 1);
> +			(*vp->func)(strchrnul(s, '=') + 1, flags);
>  
>  		if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
>  			ckfree(vp->text);
> @@ -531,7 +531,7 @@ poplocalvars(void)
>  			unsetvar(vp->text);
>  		} else {
>  			if (vp->func)
> -				(*vp->func)(strchrnul(lvp->text, '=') + 1);
> +				(*vp->func)(strchrnul(lvp->text, '=') + 1, lvp->flags);
>  			if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
>  				ckfree(vp->text);
>  			vp->flags = lvp->flags;
> diff --git a/src/var.h b/src/var.h
> index aa7575a..4329e22 100644
> --- a/src/var.h
> +++ b/src/var.h
> @@ -56,7 +56,7 @@ struct var {
>  	struct var *next;		/* next entry in hash list */
>  	int flags;			/* flags are defined above */
>  	const char *text;		/* name=value */
> -	void (*func)(const char *);
> +	void (*func)(const char *, int flags);
>  					/* function to be called when  */
>  					/* the variable gets set/unset */
>  };
> -- 
> 2.30.2

Re: [PATCH] options: don't error when unsetting OPTIND

[Steffen Nurpmeso]

Michael Greenberg wrote in
 <m2edt2nk5q.fsf@greenberg.science>:
 ...
 |Bafflingly, the POSIX spec gives the example `readonly HOME PWD`, but
 |with no explanation about why one might want to do that. (I get making
 |`HOME` readonly, but `PWD`? Is that supposed to stop `cd` from working?)

PWD is a beast with lots of "behaviors of the cd and pwd utilities
are unspecified" in case of user edits, see "2.5.3 Shell Variables".

--steffen
|
|Der Kragenbaer,                The moon bear,
|der holt sich munter           he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)

Re: [PATCH] options: don't error when unsetting OPTIND

[Michael Greenberg]

On 2022-12-14 at 06:49:46 PM, Steffen Nurpmeso wrote:

> Michael Greenberg wrote in
>  <m2edt2nk5q.fsf@greenberg.science>:
>  ...
>  |Bafflingly, the POSIX spec gives the example `readonly HOME PWD`, but
>  |with no explanation about why one might want to do that. (I get making
>  |`HOME` readonly, but `PWD`? Is that supposed to stop `cd` from working?)
>
> PWD is a beast with lots of "behaviors of the cd and pwd utilities
> are unspecified" in case of user edits, see "2.5.3 Shell Variables".

Is running `readonly PWD` the same as setting `PWD`?

The language in the documentation for `cd` and in 2.5.3 only talks about setting or
unsetting `PWD`, but not the export/readonly bits.

Re: [PATCH] options: don't error when unsetting OPTIND

[наб]

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

Hi!

On Wed, Dec 14, 2022 at 05:36:22PM -0500, Michael Greenberg wrote:
> On 2022-12-14 at 06:49:46 PM, Steffen Nurpmeso wrote:
> 
> > Michael Greenberg wrote in
> >  <m2edt2nk5q.fsf@greenberg.science>:
> >  ...
> >  |Bafflingly, the POSIX spec gives the example `readonly HOME PWD`, but
> >  |with no explanation about why one might want to do that. (I get making
> >  |`HOME` readonly, but `PWD`? Is that supposed to stop `cd` from working?)
> >
> > PWD is a beast with lots of "behaviors of the cd and pwd utilities
> > are unspecified" in case of user edits, see "2.5.3 Shell Variables".
> 
> Is running `readonly PWD` the same as setting `PWD`?
> 
> The language in the documentation for `cd` and in 2.5.3 only talks about setting or
> unsetting `PWD`, but not the export/readonly bits.

Isssue 8 Draft 2.1, XBD, 8.1 Environment Variable Definition para. 11
(p. 156, ll. 5381-5390) says:
  Additionally, a subset of the above variables are manipulated by shell built-in utilities outside of
  shell assignments. If an attempt is made to mark any of the following variables as readonly, then
  either the readonly utility shall reject the attempt, or readonly shall succeed but the shell can still
  modify the variables outside of assignment context, or readonly shall succeed but use of a shell
  built-in that would otherwise modify such a variable shall fail.
    LINENO
    OLDPWD
    OPTARG
    OPTIND
    PWD

I.e., readonly PWD; cd /; PWD=dupa must:
  * fail in readonly, or
  * fail in the cd, or
  * fail in the assignment
at implementer's choice. Which is just a long-winded way of saying "just
do whatever but only accept readonly if it does anything".

Dash appears to do option 2:
  $ ./dash -c 'readonly PWD; cd /'
  ./dash: 1: cd: PWD: is read only
  $ ./dash -c 'readonly OPTIND; getopts a a -a'
  ./dash: 1: getopts: OPTIND: is read only
which is fine.
OLDPWD behaves like PWD and OPTARG like OPTIND.
LINENO behaves like option 3 for obvious reasons.

(For completeness: in ibid., para. 12 (ll. 5391-5392)
 there's a provision for special treatment for any non-standard
 variable we want for our built-ins:
   Implementations may provide an implementation-defined set of additional variables which are
   manipulated by implementation-specific built-in utilities not defined in this standard. The
   readonly utility shall not reject marking these additional variables as readonly, but when marked
   readonly, those extension utilities shall either continue to modify the variables, or shall fail
   because the variable is readonly. None of the variables defined by this standard shall be in this
   implementation-defined set.)

Best,
наб

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] options: don't error when unsetting OPTIND

[Michael Greenberg]

On 2022-12-15 at 12:07:38 AM, наб wrote:

> Hi!
>
> On Wed, Dec 14, 2022 at 05:36:22PM -0500, Michael Greenberg wrote:
>> On 2022-12-14 at 06:49:46 PM, Steffen Nurpmeso wrote:
>> 
>> > Michael Greenberg wrote in
>> >  <m2edt2nk5q.fsf@greenberg.science>:
>> >  ...
>> >  |Bafflingly, the POSIX spec gives the example `readonly HOME PWD`, but
>> >  |with no explanation about why one might want to do that. (I get making
>> >  |`HOME` readonly, but `PWD`? Is that supposed to stop `cd` from working?)
>> >
>> > PWD is a beast with lots of "behaviors of the cd and pwd utilities
>> > are unspecified" in case of user edits, see "2.5.3 Shell Variables".
>> 
>> Is running `readonly PWD` the same as setting `PWD`?
>> 
>> The language in the documentation for `cd` and in 2.5.3 only talks about setting or
>> unsetting `PWD`, but not the export/readonly bits.
>
> Isssue 8 Draft 2.1, XBD, 8.1 Environment Variable Definition para. 11
> (p. 156, ll. 5381-5390) says:

Thank you for finding this! I didn't think to check the current draft.

Cheers,
Michael

[PATCH] var: Do not add 1 to return value of strchrnul

[Herbert Xu]

наб <nabijaczleweli@nabijaczleweli.xyz> wrote:
>
> --- a/src/var.c
> +++ b/src/var.c
> @@ -266,7 +266,7 @@ struct var *setvareq(char *s, int flags)
>                        goto out;
> 
>                if (vp->func && (flags & VNOFUNC) == 0)
> -                       (*vp->func)(strchrnul(s, '=') + 1);
> +                       (*vp->func)(strchrnul(s, '=') + 1, flags);

Yes this was definitely broken.  strchrnul returns a pointer to the
final NUL character so adding one to it is just wrong.

However, I don't think we need to pass the flags to the action
function since none of them care about whether it's unset.  We
just need to pass a pointer to an empty string rather than some
bogus pointer.

---8<---
When a variable like OPTIND is unset dash may call the action
function with a bogus pointer because it tries to add one to
the return value of strchrnul unconditionally.

Use strchr and nullstr instead.

Link: https://bugs.debian.org/985478
Reported-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/var.c b/src/var.c
index ef9c2bd..f42bfd7 100644
--- a/src/var.c
+++ b/src/var.c
@@ -266,7 +266,7 @@ struct var *setvareq(char *s, int flags)
 			goto out;
 
 		if (vp->func && (flags & VNOFUNC) == 0)
-			(*vp->func)(strchrnul(s, '=') + 1);
+			(*vp->func)((strchr(s, '=') ?: nullstr - 1) + 1);
 
 		if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 			ckfree(vp->text);
@@ -531,7 +531,8 @@ poplocalvars(void)
 			unsetvar(vp->text);
 		} else {
 			if (vp->func)
-				(*vp->func)(strchrnul(lvp->text, '=') + 1);
+				(*vp->func)((strchr(lvp->text, '=') ?:
+					     nullstr - 1) + 1);
 			if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 				ckfree(vp->text);
 			vp->flags = lvp->flags;
-- 
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

[v2 PATCH] var: Do not add 1 to return value of strchrnul

[Herbert Xu]

When a variable like OPTIND is unset dash may call the action
function with a bogus pointer because it tries to add one to
the return value of strchrnul unconditionally.

Use strchr and nullstr instead.

Link: https://bugs.debian.org/985478
Reported-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/var.c b/src/var.c
index ef9c2bd..b70d72c 100644
--- a/src/var.c
+++ b/src/var.c
@@ -154,6 +154,10 @@ RESET {
 }
 #endif
 
+static char *varnull(const char *s)
+{
+	return (strchr(s, '=') ?: nullstr - 1) + 1;
+}
 
 /*
  * This routine initializes the builtin variables.  It is called when the
@@ -266,7 +270,7 @@ struct var *setvareq(char *s, int flags)
 			goto out;
 
 		if (vp->func && (flags & VNOFUNC) == 0)
-			(*vp->func)(strchrnul(s, '=') + 1);
+			(*vp->func)(varnull(s));
 
 		if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 			ckfree(vp->text);
@@ -531,7 +535,7 @@ poplocalvars(void)
 			unsetvar(vp->text);
 		} else {
 			if (vp->func)
-				(*vp->func)(strchrnul(lvp->text, '=') + 1);
+				(*vp->func)(varnull(lvp->text));
 			if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 				ckfree(vp->text);
 			vp->flags = lvp->flags;
-- 
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

Re: [PATCH] var: Do not add 1 to return value of strchrnul

[наб]

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

Hi!

On Tue, Jan 03, 2023 at 05:39:41PM +0800, Herbert Xu wrote:
> наб <nabijaczleweli@nabijaczleweli.xyz> wrote:
> > --- a/src/var.c
> > +++ b/src/var.c
> > @@ -266,7 +266,7 @@ struct var *setvareq(char *s, int flags)
> >                        goto out;
> > 
> >                if (vp->func && (flags & VNOFUNC) == 0)
> > -                       (*vp->func)(strchrnul(s, '=') + 1);
> > +                       (*vp->func)(strchrnul(s, '=') + 1, flags);
> Yes this was definitely broken.  strchrnul returns a pointer to the
> final NUL character so adding one to it is just wrong.
That originally gave me pause but I assumed this is an intentional
muddling for a known memory lay-out or w/e,
but that explains the corruption I saw sometimes, yeah.

> However, I don't think we need to pass the flags to the action
> function since none of them care about whether it's unset.  We
> just need to pass a pointer to an empty string rather than some
> bogus pointer.
This, unsurprisingly, produces the same error, because getoptsreset()
needs a valid number for OPTIND:
  $ src/dash -c 'unset OPTIND'
  src/dash: 1: unset: Illegal number:
and the empty string isn't a valid number
(in the simple case of src/dash -c 'unset OPTIND' &a. the original
 broken approach also produced an empty string on accident
 in most cases for me).

We do need to at least to /some/how signal to getoptsreset() that we're
unsetting while preserving the requirement that OPTIND is an integer,
and passing a flag is the only sensible way, I think.

Scissor-patch rebased on top of your v2 below.

Best,
-- >8 --
Subject: [PATCH] options: don't error when unsetting OPTIND

unset OPTIND ends up calling getoptsreset("") which errors out with
  sh: 1: unset: Illegal number:

Pass the current flags to struct var->func, set the getopts optind to 1
and continue with allowing the unset.

We still forbid OPTIND=, OPTIND=-1, OPTIND=abc, &c.

Fixes: https://bugs.debian.org/985478
---
 src/exec.c    | 2 +-
 src/exec.h    | 2 +-
 src/mail.c    | 2 +-
 src/mail.h    | 2 +-
 src/options.c | 5 ++---
 src/options.h | 2 +-
 src/var.c     | 4 ++--
 src/var.h     | 2 +-
 8 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/exec.c b/src/exec.c
index 87354d4..68fa8ab 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -565,7 +565,7 @@ hashcd(void)
  */
 
 void
-changepath(const char *newval)
+changepath(const char *newval, int unused)
 {
 	const char *new;
 	int idx;
diff --git a/src/exec.h b/src/exec.h
index 423b07e..0f74be4 100644
--- a/src/exec.h
+++ b/src/exec.h
@@ -69,7 +69,7 @@ int hashcmd(int, char **);
 void find_command(char *, struct cmdentry *, int, const char *);
 struct builtincmd *find_builtin(const char *);
 void hashcd(void);
-void changepath(const char *);
+void changepath(const char *, int);
 #ifdef notdef
 void getcmdentry(char *, struct cmdentry *);
 #endif
diff --git a/src/mail.c b/src/mail.c
index 8eacb2d..e81d2b4 100644
--- a/src/mail.c
+++ b/src/mail.c
@@ -109,7 +109,7 @@ chkmail(void)
 
 
 void
-changemail(const char *val)
+changemail(const char *val, int unused)
 {
 	changed++;
 }
diff --git a/src/mail.h b/src/mail.h
index 3c6b21d..70b54a4 100644
--- a/src/mail.h
+++ b/src/mail.h
@@ -35,4 +35,4 @@
  */
 
 void chkmail(void);
-void changemail(const char *);
+void changemail(const char *, int);
diff --git a/src/options.c b/src/options.c
index a46c23b..81f2c4b 100644
--- a/src/options.c
+++ b/src/options.c
@@ -390,10 +390,9 @@ setcmd(int argc, char **argv)
 
 
 void
-getoptsreset(value)
-	const char *value;
+getoptsreset(const char *value, int flags)
 {
-	shellparam.optind = number(value) ?: 1;
+	shellparam.optind = (flags & VUNSET) ? 1 : number(value) ?: 1;
 	shellparam.optoff = -1;
 }
 
diff --git a/src/options.h b/src/options.h
index 975fe33..10bcb88 100644
--- a/src/options.h
+++ b/src/options.h
@@ -83,4 +83,4 @@ int shiftcmd(int, char **);
 int setcmd(int, char **);
 int getoptscmd(int, char **);
 int nextopt(const char *);
-void getoptsreset(const char *);
+void getoptsreset(const char *, int);
diff --git a/src/var.c b/src/var.c
index b70d72c..13a946b 100644
--- a/src/var.c
+++ b/src/var.c
@@ -270,7 +270,7 @@ struct var *setvareq(char *s, int flags)
 			goto out;
 
 		if (vp->func && (flags & VNOFUNC) == 0)
-			(*vp->func)(varnull(s));
+			(*vp->func)(varnull(s), flags);
 
 		if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 			ckfree(vp->text);
@@ -535,7 +535,7 @@ poplocalvars(void)
 			unsetvar(vp->text);
 		} else {
 			if (vp->func)
-				(*vp->func)(varnull(lvp->text));
+				(*vp->func)(varnull(lvp->text), lvp->flags);
 			if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 				ckfree(vp->text);
 			vp->flags = lvp->flags;
diff --git a/src/var.h b/src/var.h
index aa7575a..4329e22 100644
--- a/src/var.h
+++ b/src/var.h
@@ -56,7 +56,7 @@ struct var {
 	struct var *next;		/* next entry in hash list */
 	int flags;			/* flags are defined above */
 	const char *text;		/* name=value */
-	void (*func)(const char *);
+	void (*func)(const char *, int flags);
 					/* function to be called when  */
 					/* the variable gets set/unset */
 };
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] var: Do not add 1 to return value of strchrnul

[Herbert Xu]

On Tue, Jan 03, 2023 at 01:26:19PM +0100, наб wrote:
>
> > However, I don't think we need to pass the flags to the action
> > function since none of them care about whether it's unset.  We
> > just need to pass a pointer to an empty string rather than some
> > bogus pointer.
> This, unsurprisingly, produces the same error, because getoptsreset()
> needs a valid number for OPTIND:
>   $ src/dash -c 'unset OPTIND'
>   src/dash: 1: unset: Illegal number:

You get the same result with OPTIND="" so I don't see the issue.

Am I missing something with respect to POSIX?

Thanks,
-- 
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

Re: [PATCH] var: Do not add 1 to return value of strchrnul

[наб]

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

Hi!

On Wed, Jan 04, 2023 at 05:59:47PM +0800, Herbert Xu wrote:
> On Tue, Jan 03, 2023 at 01:26:19PM +0100, наб wrote:
> > > However, I don't think we need to pass the flags to the action
> > > function since none of them care about whether it's unset.  We
> > > just need to pass a pointer to an empty string rather than some
> > > bogus pointer.
> > This, unsurprisingly, produces the same error, because getoptsreset()
> > needs a valid number for OPTIND:
> >   $ src/dash -c 'unset OPTIND'
> >   src/dash: 1: unset: Illegal number:
> You get the same result with OPTIND="" so I don't see the issue.
Which is an unrelated operation, and obviously nonsensical.
It's good that it fails.

> Am I missing something with respect to POSIX?
AFAICT, after trawling through the Issue 8 Draft 2.1,
there are no special provisions for unset/OPTIND,
like there are for readonly.
(See the other branch of this thread, but to summarise:
 setting LINENO, OLDPWD, OPTARG, OPTIND, or PWD readonly /must/ fail in:
 (a) readonly xor (b) the builtin that edits it xor (c) on assignment.)
This indicates that "unset OPTIND" must behave as-specified,
i.e. unset OPTIND and remove it from the environment.

In dash, this really only affects OPTIND, since the only other special
variable that parses an int is sethistsize(), which explicitly has
-- >8 --
        if (hist != NULL) {
                if (hs == NULL || *hs == '\0' ||
                   (histsize = atoi(hs)) < 0)
                        histsize = 100;
                history(hist, &he, H_SETSIZE, histsize);
        }
-- >8 --
which considers HISTSIZE= and unset HISTSIZE to be equivalent;
the only other setter function that inspects the new value is for PATH,
and working out to the empty string works for that case.

We could also pass NULL when unsetting, I guess, since sethistsize() is
already equipped for it, if you prefer to not have a flags argument?

Best,
наб

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] var: Do not add 1 to return value of strchrnul

[Herbert Xu]

On Wed, Jan 04, 2023 at 01:54:21PM +0100, наб wrote:
> 
> AFAICT, after trawling through the Issue 8 Draft 2.1,
> there are no special provisions for unset/OPTIND,

Well it says this regarding OPTIND:

: If the application sets OPTIND to the value 1, a new set of parameters
: can be used: either the current positional parameters or new arg
: values. Any other attempt to invoke getopts multiple times in a single
: shell execution environment with parameters (positional parameters
: or arg operands) that are not the same in all invocations, or with an
: OPTIND value modified to be a value other than 1, produces unspecified
: results.

Unsetting OPTIND ought to do the same thing as setting it to "",
which produces unspecified results and I see nothing wrong with
printing an error in this case.

Cheers,
-- 
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

Re: [PATCH] var: Do not add 1 to return value of strchrnul

[наб]

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

Hi!

On Wed, Jan 04, 2023 at 10:12:36PM +0800, Herbert Xu wrote:
> On Wed, Jan 04, 2023 at 01:54:21PM +0100, наб wrote:
> > AFAICT, after trawling through the Issue 8 Draft 2.1,
> > there are no special provisions for unset/OPTIND,
> Well it says this regarding OPTIND:
> 
> : If the application sets OPTIND to the value 1, a new set of parameters
> : can be used: either the current positional parameters or new arg
> : values. Any other attempt to invoke getopts multiple times in a single
> : shell execution environment with parameters (positional parameters
> : or arg operands) that are not the same in all invocations, or with an
> : OPTIND value modified to be a value other than 1, produces unspecified
> : results.
> 
> Unsetting OPTIND ought to do the same thing as setting it to "",
On the basis of? Those are fundamentally unrelated operations,
and when they are to do the same thing, this is explicitly noted.

Additionally, this is a description of /getopts/, and therefore only
applies if you do "OPTIND=dupa; getopts ..." ‒ the getopts call is
unspecified.

/Any/ operation on OPTIND is legal, and the current behaviour is wrong
(but it seems that no-one has actually cared) ‒ POSIX defines this in
terms of getopts inspecting OPTIND when it's run.

"OPTIND=whatever" is legal and must result in OPTIND becoming whatever.
bash, ksh, and zsh all reduce it to OPTIND=0, so it appears that no-one
actually cares about this so erroring out is fine, I guess.
(mksh does the same but folds to 1; posh refuses the assignment),

"unset OPTIND" is legal, and subsequent getopts calls are well-defined
(it's unclear to me what it oughta do /per se/, but it's not "been set",
 so it's unclear what it's well-defined /to/, but since "the index of
 the next argument to be processed in the shell variable OPTIND" and
 that value doesn't exist, erroring out is fine I guess),
and all the above shells agree
(as does mksh; posh ignores the unset).
and this fell out of a debbug where someone used it
(and additionally cites a few more shells),
so people actually care about this behaviour.

In testing the poster's shells,
I found that yash behaves close to correct here:
-- >8 --
nabijaczleweli@tarta dash 2$ OPTIND=dupa
nabijaczleweli@tarta dash 2$ getopts a a a
getopts: $OPTIND has an invalid value
nabijaczleweli@tarta dash 1 2$ OPTIND=300
nabijaczleweli@tarta dash 2$ getopts a a a
nabijaczleweli@tarta dash 1 2$ unset OPTIND
nabijaczleweli@tarta dash 2$ set | grep -i optind
nabijaczleweli@tarta dash 1 2$ getopts a a a
getopts: $OPTIND has an invalid value
-- >8 --

As does busybox ash:
-- >8 --
~/code/dash $ OPTIND=dupa
~/code/dash $ set | grep -i optind
OPTIND='dupa'
~/code/dash $ getopts a a a
~/code/dash $ set | grep -i optind
OPTIND='1'
~/code/dash $ unset OPTIND
~/code/dash $ set | grep -i optind
_='OPTIND'
~/code/dash $ getopts a a a
~/code/dash $ set | grep -i optind
OPTIND='1'
-- >8 --

The minimal change required to achieve compatibility is my patch,
or some equivalent variant of it, like passing NULL instead of flags.
The minimal change required to achieve conformance would be
to get parse OPTIND in getopts like yash.

There's code in the wild that uses "unset OPTIND; getopts ..."
instead of "OPTIND=1; getopts ...":
  https://codesearch.debian.net/search?q=unset+OPTIND&literal=1&perpkg=1
but those are all bash programs, so it's up to you whether they should
receive explicit provisions for this.

I'm including a PoC scissor-patch below; in particular the ternary
should either be turned into an equivalent of ': "1"' if you want to
support "unset OPTIND; getopts ..." or produce a better error than
"./dash: 1: getopts: Illegal number: unset", but you get the idea
(and shellparam should get fully boolified
 and dash.1 wants to have an explicit note that reset is OPTIND=1,
 but I can follow up with that if we're in agreement here).

Best,
-- >8 --
Subject: [PATCH] options: lazy-parse OPTIND in getopts, per POSIX

This allows "unset OPTIND" and "OPTIND=dupa" to execute as-written
(and as required by POSIX), and catches them as an error in getopts,
which exits 2 ("An error occurred.").

Fixes: https://bugs.debian.org/985478
---
 src/options.c | 30 +++++++++++++++++-------------
 src/options.h |  5 ++++-
 src/var.h     |  1 +
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/options.c b/src/options.c
index a46c23b..578a13f 100644
--- a/src/options.c
+++ b/src/options.c
@@ -164,7 +164,7 @@ setarg0:
 	shellparam.p = xargv;
 	shellparam.optind = 1;
 	shellparam.optoff = -1;
-	/* assert(shellparam.malloc == 0 && shellparam.nparam == 0); */
+	/* assert(!shellparam.malloc && !shellparam.optreparse && shellparam.nparam == 0); */
 	while (*xargv) {
 		shellparam.nparam++;
 		xargv++;
@@ -390,11 +390,9 @@ setcmd(int argc, char **argv)
 
 
 void
-getoptsreset(value)
-	const char *value;
+getoptsreset(const char *value)
 {
-	shellparam.optind = number(value) ?: 1;
-	shellparam.optoff = -1;
+	shellparam.optreparse = true;
 }
 
 /*
@@ -408,22 +406,28 @@ int
 getoptscmd(int argc, char **argv)
 {
 	char **optbase;
+	unsigned max;
 
 	if (argc < 3)
 		sh_error("Usage: getopts optstring var [arg]");
 	else if (argc == 3) {
 		optbase = shellparam.p;
-		if ((unsigned)shellparam.optind > shellparam.nparam + 1) {
-			shellparam.optind = 1;
-			shellparam.optoff = -1;
-		}
+		max = shellparam.nparam + 1;
 	}
 	else {
 		optbase = &argv[3];
-		if ((unsigned)shellparam.optind > argc - 2) {
-			shellparam.optind = 1;
-			shellparam.optoff = -1;
-		}
+		max = argc - 2;
+	}
+
+	if (shellparam.optreparse) {
+		shellparam.optind = number(optindset() ? optindval() : "unset") ?: 1;
+		shellparam.optoff = -1;
+		shellparam.optreparse = false;
+	}
+
+	if ((unsigned)shellparam.optind > max) {
+		shellparam.optind = 1;
+		shellparam.optoff = -1;
 	}
 
 	return getopts(argv[1], argv[2], optbase);
diff --git a/src/options.h b/src/options.h
index 975fe33..8aebf9f 100644
--- a/src/options.h
+++ b/src/options.h
@@ -34,9 +34,12 @@
  *	@(#)options.h	8.2 (Berkeley) 5/4/95
  */
 
+#include <stdbool.h>
+
 struct shparam {
 	int nparam;		/* # of positional parameters (without $0) */
-	unsigned char malloc;	/* if parameter list dynamically allocated */
+	bool malloc;		/* if parameter list dynamically allocated */
+	bool optreparse;	/* re-parse optind/optoff from $OPTIND */
 	char **p;		/* parameter list */
 	int optind;		/* next parameter to be processed by getopts */
 	int optoff;		/* used by getopts */
diff --git a/src/var.h b/src/var.h
index aa7575a..ca90a68 100644
--- a/src/var.h
+++ b/src/var.h
@@ -133,6 +133,7 @@ extern char linenovar[];
 #define attyset()	((vatty.flags & VUNSET) == 0)
 #endif
 #define mpathset()	((vmpath.flags & VUNSET) == 0)
+#define optindset()	((voptind.flags & VUNSET) == 0)
 
 void initvar(void);
 struct var *setvar(const char *name, const char *val, int flags);
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] var: Do not add 1 to return value of strchrnul

[Harald van Dijk]

On 04/01/2023 16:05, наб wrote:
> Hi!
> 
> On Wed, Jan 04, 2023 at 10:12:36PM +0800, Herbert Xu wrote:
>> On Wed, Jan 04, 2023 at 01:54:21PM +0100, наб wrote:
>>> AFAICT, after trawling through the Issue 8 Draft 2.1,
>>> there are no special provisions for unset/OPTIND,
>> Well it says this regarding OPTIND:
>>
>> : If the application sets OPTIND to the value 1, a new set of parameters
>> : can be used: either the current positional parameters or new arg
>> : values. Any other attempt to invoke getopts multiple times in a single
>> : shell execution environment with parameters (positional parameters
>> : or arg operands) that are not the same in all invocations, or with an
>> : OPTIND value modified to be a value other than 1, produces unspecified
>> : results.
>>
>> Unsetting OPTIND ought to do the same thing as setting it to "",
> On the basis of? Those are fundamentally unrelated operations,
> and when they are to do the same thing, this is explicitly noted.

On the basis of common sense, keeping in mind that different people's 
ideas of common sense will be different. In this one, I'm tempted to 
agree with Herbert Xu, $OPTIND would expand to an empty string whether 
OPTIND is unset or set to an empty string, assuming set -u is not in 
effect, so for the purpose of the getopts command it would be good for 
it to behave the same way there too.

> Additionally, this is a description of /getopts/, and therefore only
> applies if you do "OPTIND=dupa; getopts ..." ‒ the getopts call is
> unspecified.
> 
> /Any/ operation on OPTIND is legal, and the current behaviour is wrong
> (but it seems that no-one has actually cared) ‒ POSIX defines this in
> terms of getopts inspecting OPTIND when it's run.

Indeed, but dash isn't the only shell that behaves this way (posh does 
too) and I'm not sure whether this is what POSIX intends to specify. I 
agree with your interpretation of what POSIX actually says, but it would 
be good for POSIX to reword it regardless of what is intended to be 
clearer that this case was actually considered.

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.

Cheers,
Harald van Dijk

[PATCH] options: Always reset OPTIND in getoptsreset

[Herbert Xu]

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: наб <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;
 }
 
-- 
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

Re: [PATCH] options: Always reset OPTIND in getoptsreset

[Harald van Dijk]

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

[v2 PATCH] options: Always reset OPTIND in getoptsreset

[Herbert Xu]

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: наб <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

Re: [v2 PATCH] options: Always reset OPTIND in getoptsreset

[наб]

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

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]