Hi!
On Tue, Jan 03, 2023 at 05:39:41PM +0800, Herbert Xu wrote:
> наб <nabijaczleweli@xxxxxxxxxxxxxxxxxx> 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:
signature.asc
Description: PGP signature
