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

[Michael Greenberg <michael@greenberg.science>]

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