Seconded! A test suite for dash would have saved me a great deal of
trouble. I have a number of tests I've written that I'd be happy to
contribute.
Is dash ever tested on the POSIX test suite? Certification is expensive,
but their test suite seems to be quite thorough... and it may even be
possible to use their tests even if dash isn't going to be officially
POSIX compliant.
Cheers,
Michael
On 2018-11-15 9:03 AM, Devin Hussey wrote:
> From 65f9c258658b50aeb67947f5b54e926538560488 Mon Sep 17 00:00:00 2001
> From: Devin Hussey <husseydevin@xxxxxxxxx>
> Date: Thu, 15 Nov 2018 08:29:33 -0500
> Subject: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
>
> They are not ready, as simply running dash on its own configure
> script will reveal (note: config.cache does not exist):
>
> checking for a BSD-compatible install... (cached)
> checking whether build environment is sane... yes
> configure: 1: eval: --is-lightweight: not found
> configure: WARNING: 'missing' script is too old or missing
> checking for a thread-safe mkdir -p... (cached) -p
> checking for gawk... (cached) no
> checking for mawk... (cached) no
> checking for nawk... (cached) no
> checking for awk... (cached) no
> checking whether make sets $(MAKE)... (cached) configure: 1: test:
> =: unexpected operator
> no
> checking whether make supports nested variables... (cached)
> configure: 2878: test: =: unexpected operator
> checking for gcc... (cached) no
> checking for cc... (cached) no
> checking for cl.exe... (cached) no
> configure: error: in `/Users/Devin/dash2':
> configure: error: no acceptable C compiler found in $PATH
> See `config.log' for more details
>
> 3cd5386 completely broke expansion.
>
> I got the configure script to run by changing
>
> return p - 1;
>
> to
>
> return p;
>
> in argstr, but variable expansion is still broken.
>
> The most broken thing is ${FOO+BAR}:
>
> $ bash -c 'unset FOO; echo ${FOO+BAR}'
>
> $ ./dash -c 'unset FOO; echo ${FOO+BAR}'
> BAR
> $
>
> We should really set up a test suite, as that isn't the first
> time some obvious bugs have made their way into the code base.
> I wonder if FreeBSD will let us use their tests, as they have
> a very (too?) expansive set of tests.
>
> At the very least, we should make sure dash can run its own
> configure script, as that is the most common shell script
> to run.
>
> Signed-off-by: Devin Hussey <husseydevin@xxxxxxxxx>
> ---
> src/expand.c | 321 ++++++++++++++++++++++++++++-----------------------
> src/expand.h | 2 +-
> 2 files changed, 176 insertions(+), 147 deletions(-)
>
> diff --git a/src/expand.c b/src/expand.c
> index 14daa63..7a51766 100644
> --- a/src/expand.c
> +++ b/src/expand.c
> @@ -110,13 +110,13 @@ static struct ifsregion *ifslastp;
> /* holds expanded arg list */
> static struct arglist exparg;
>
> -static char *argstr(char *p, int flag);
> -static char *exptilde(char *startp, int flag);
> -static char *expari(char *start, int flag);
> +STATIC void argstr(char *, int);
> +STATIC char *exptilde(char *, char *, int);
> STATIC void expbackq(union node *, int);
> +STATIC const char *subevalvar(char *, char *, int, int, int, int, int);
> STATIC char *evalvar(char *, int);
> static size_t strtodest(const char *p, int flags);
> -static size_t memtodest(const char *p, size_t len, int flags);
> +static void memtodest(const char *p, size_t len, int flags);
> STATIC ssize_t varvalue(char *, int, int, int);
> STATIC void expandmeta(struct strlist *, int);
> #ifdef HAVE_GLOB
> @@ -133,7 +133,7 @@ STATIC int pmatch(const char *, const char *);
> #else
> #define pmatch(a, b) !fnmatch((a), (b), 0)
> #endif
> -static size_t cvtnum(intmax_t num, int flags);
> +STATIC int cvtnum(intmax_t);
> STATIC size_t esclen(const char *, const char *);
> STATIC char *scanleft(char *, char *, char *, char *, int, int);
> STATIC char *scanright(char *, char *, char *, char *, int, int);
> @@ -192,11 +192,13 @@ expandarg(union node *arg, struct arglist
> *arglist, int flag)
> argbackq = arg->narg.backquote;
> STARTSTACKSTR(expdest);
> argstr(arg->narg.text, flag);
> + p = _STPUTC('\0', expdest);
> + expdest = p - 1;
> if (arglist == NULL) {
> /* here document expanded */
> goto out;
> }
> - p = grabstackstr(expdest);
> + p = grabstackstr(p);
> exparg.lastp = &exparg.list;
> /*
> * TODO - EXP_REDIR
> @@ -230,7 +232,8 @@ out:
> * $@ like $* since no splitting will be performed.
> */
>
> -static char *argstr(char *p, int flag)
> +STATIC void
> +argstr(char *p, int flag)
> {
> static const char spclchars[] = {
> '=',
> @@ -240,7 +243,6 @@ static char *argstr(char *p, int flag)
> CTLESC,
> CTLVAR,
> CTLBACKQ,
> - CTLARI,
> CTLENDARI,
> 0
> };
> @@ -251,41 +253,35 @@ static char *argstr(char *p, int flag)
> size_t length;
> int startloc;
>
> - reject += !!(flag & EXP_VARTILDE2);
> - reject += flag & EXP_VARTILDE ? 0 : 2;
> + if (!(flag & EXP_VARTILDE)) {
> + reject += 2;
> + } else if (flag & EXP_VARTILDE2) {
> + reject++;
> + }
> inquotes = 0;
> length = 0;
> if (flag & EXP_TILDE) {
> + char *q;
> +
> flag &= ~EXP_TILDE;
> tilde:
> - if (*p == '~')
> - p = exptilde(p, flag);
> + q = p;
> + if (*q == '~')
> + p = exptilde(p, q, flag);
> }
> start:
> startloc = expdest - (char *)stackblock();
> for (;;) {
> - int end;
> -
> length += strcspn(p + length, reject);
> - end = 0;
> c = (signed char)p[length];
> - if (!(c & 0x80) || c == CTLENDARI || c == CTLENDVAR) {
> - /*
> - * c == '=' || c == ':' || c == '\0' ||
> - * c == CTLENDARI || c == CTLENDVAR
> - */
> + if (c && (!(c & 0x80) || c == CTLENDARI)) {
> + /* c == '=' || c == ':' || c == CTLENDARI */
> length++;
> - /* c == '\0' || c == CTLENDARI || c == CTLENDVAR */
> - end = !!((c - 1) & 0x80);
> }
> - if (length > 0 && !(flag & EXP_DISCARD)) {
> + if (length > 0) {
> int newloc;
> - char *q;
> -
> - q = stnputs(p, length, expdest);
> - q[-1] &= end - 1;
> - expdest = q - (flag & EXP_WORD ? end : 0);
> - newloc = expdest - (char *)stackblock() - end;
> + expdest = stnputs(p, length, expdest);
> + newloc = expdest - (char *)stackblock();
> if (breakall && !inquotes && newloc > startloc) {
> recordregion(startloc, newloc, 0);
> }
> @@ -294,11 +290,14 @@ start:
> p += length + 1;
> length = 0;
>
> - if (end)
> - break;
> -
> switch (c) {
> + case '\0':
> + goto breakloop;
> case '=':
> + if (flag & EXP_VARTILDE2) {
> + p--;
> + continue;
> + }
> flag |= EXP_VARTILDE2;
> reject++;
> /* fall through */
> @@ -311,6 +310,11 @@ start:
> goto tilde;
> }
> continue;
> + }
> +
> + switch (c) {
> + case CTLENDVAR: /* ??? */
> + goto breakloop;
> case CTLQUOTEMARK:
> /* "$@" syntax adherence hack */
> if (!inquotes && !memcmp(p, dolatstr + 1,
> @@ -335,23 +339,25 @@ addquote:
> goto start;
> case CTLBACKQ:
> expbackq(argbackq->n, flag | inquotes);
> + argbackq = argbackq->next;
> goto start;
> - case CTLARI:
> - p = expari(p, flag | inquotes);
> + case CTLENDARI:
> + p--;
> + expari(flag | inquotes);
> goto start;
> }
> }
> - return p - 1;
> +breakloop:
> + ;
> }
>
> -static char *exptilde(char *startp, int flag)
> +STATIC char *
> +exptilde(char *startp, char *p, int flag)
> {
> signed char c;
> char *name;
> const char *home;
> - char *p;
>
> - p = startp;
> name = p + 1;
>
> while ((c = *++p) != '\0') {
> @@ -370,21 +376,19 @@ static char *exptilde(char *startp, int flag)
> }
> }
> done:
> - if (flag & EXP_DISCARD)
> - goto out;
> *p = '\0';
> if (*name == '\0') {
> home = lookupvar(homestr);
> } else {
> home = getpwhome(name);
> }
> - *p = c;
> if (!home)
> goto lose;
> + *p = c;
> strtodest(home, flag | EXP_QUOTED);
> -out:
> return (p);
> lose:
> + *p = c;
> return (startp);
> }
>
> @@ -433,43 +437,63 @@ removerecordregions(int endoff)
> * Expand arithmetic expression. Backup to start of expression,
> * evaluate, place result in (backed up) result, adjust string position.
> */
> -static char *expari(char *start, int flag)
> +void
> +expari(int flag)
> {
> struct stackmark sm;
> + char *p, *start;
> int begoff;
> - int endoff;
> int len;
> intmax_t result;
> - char *p;
>
> - p = stackblock();
> - begoff = expdest - p;
> - p = argstr(start, flag & EXP_DISCARD);
> -
> - if (flag & EXP_DISCARD)
> - goto out;
> + /* ifsfree(); */
>
> + /*
> + * This routine is slightly over-complicated for
> + * efficiency. Next we scan backwards looking for the
> + * start of arithmetic.
> + */
> start = stackblock();
> - endoff = expdest - start;
> - start += begoff;
> - STADJUST(start - expdest, expdest);
> + p = expdest;
> + pushstackmark(&sm, p - start);
> + *--p = '\0';
> + p--;
> + do {
> + int esc;
> +
> + while (*p != (char)CTLARI) {
> + p--;
> +#ifdef DEBUG
> + if (p < start) {
> + sh_error("missing CTLARI (shouldn't happen)");
> + }
> +#endif
> + }
> +
> + esc = esclen(start, p);
> + if (!(esc % 2)) {
> + break;
> + }
> +
> + p -= esc + 1;
> + } while (1);
> +
> + begoff = p - start;
>
> removerecordregions(begoff);
>
> + expdest = p;
> +
> if (likely(flag & QUOTES_ESC))
> - rmescapes(start);
> + rmescapes(p + 1);
>
> - pushstackmark(&sm, endoff);
> - result = arith(start);
> + result = arith(p + 1);
> popstackmark(&sm);
>
> - len = cvtnum(result, flag);
> + len = cvtnum(result);
>
> if (likely(!(flag & EXP_QUOTED)))
> recordregion(begoff, begoff + len, 0);
> -
> -out:
> - return p;
> }
>
>
> @@ -488,9 +512,6 @@ expbackq(union node *cmd, int flag)
> int startloc;
> struct stackmark smark;
>
> - if (flag & EXP_DISCARD)
> - goto out;
> -
> INTOFF;
> startloc = expdest - (char *)stackblock();
> pushstackmark(&smark, startloc);
> @@ -535,9 +556,6 @@ read:
> (dest - (char *)stackblock()) - startloc,
> (dest - (char *)stackblock()) - startloc,
> stackblock() + startloc));
> -
> -out:
> - argbackq = argbackq->next;
> }
>
>
> @@ -608,35 +626,33 @@ scanright(
> return 0;
> }
>
> -static char *subevalvar(char *start, char *str, int strloc, int startloc,
> - int varflags, int flag)
> +STATIC const char *
> +subevalvar(char *p, char *str, int strloc, int subtype, int startloc,
> int varflags, int flag)
> {
> - int subtype = varflags & VSTYPE;
> int quotes = flag & QUOTES_ESC;
> char *startp;
> char *loc;
> - long amount;
> + struct nodelist *saveargbackq = argbackq;
> + int amount;
> char *rmesc, *rmescend;
> int zero;
> char *(*scan)(char *, char *, char *, char *, int , int);
> - char *p;
> -
> - p = argstr(start, (flag & EXP_DISCARD) | EXP_TILDE |
> - (str ? 0 : EXP_CASE));
> - if (flag & EXP_DISCARD)
> - return p;
>
> + argstr(p, EXP_TILDE | (subtype != VSASSIGN && subtype != VSQUESTION ?
> + EXP_CASE : 0));
> + STPUTC('\0', expdest);
> + argbackq = saveargbackq;
> startp = stackblock() + startloc;
>
> switch (subtype) {
> case VSASSIGN:
> setvar(str, startp, 0);
> -
> - loc = startp;
> - goto out;
> + amount = startp - expdest;
> + STADJUST(amount, expdest);
> + return startp;
>
> case VSQUESTION:
> - varunset(start, str, startp, varflags);
> + varunset(p, str, startp, varflags);
> /* NOTREACHED */
> }
>
> @@ -671,17 +687,10 @@ static char *subevalvar(char *start, char *str,
> int strloc, int startloc,
> loc = startp + (str - loc) - 1;
> }
> *loc = '\0';
> - } else
> - loc = str - 1;
> -
> -out:
> - amount = loc - expdest;
> - STADJUST(amount, expdest);
> -
> - /* Remove any recorded regions beyond start of variable */
> - removerecordregions(startloc);
> -
> - return p;
> + amount = loc - expdest;
> + STADJUST(amount, expdest);
> + }
> + return loc;
> }
>
>
> @@ -696,6 +705,7 @@ evalvar(char *p, int flag)
> int varflags;
> char *var;
> int patloc;
> + int c;
> int startloc;
> ssize_t varlen;
> int quoted;
> @@ -703,6 +713,9 @@ evalvar(char *p, int flag)
> varflags = *p++;
> subtype = varflags & VSTYPE;
>
> + if (!subtype)
> + sh_error("Bad substitution");
> +
> quoted = flag & EXP_QUOTED;
> var = p;
> startloc = expdest - (char *)stackblock();
> @@ -713,30 +726,32 @@ again:
> if (varflags & VSNUL)
> varlen--;
>
> - switch (subtype) {
> - case VSPLUS:
> + if (subtype == VSPLUS) {
> varlen = -1 - varlen;
> - /* fall through */
> + goto vsplus;
> + }
>
> - case 0:
> - case VSMINUS:
> - p = argstr(p, flag | EXP_TILDE | EXP_WORD);
> - if (varlen < 0)
> - return p;
> + if (subtype == VSMINUS) {
> +vsplus:
> + if (varlen < 0) {
> + argstr(p, flag | EXP_TILDE | EXP_WORD);
> + goto end;
> + }
> goto record;
> + }
>
> - case VSASSIGN:
> - case VSQUESTION:
> + if (subtype == VSASSIGN || subtype == VSQUESTION) {
> if (varlen >= 0)
> goto record;
>
> - p = subevalvar(p, var, 0, startloc, varflags,
> - flag & ~QUOTES_ESC);
> -
> - if (flag & EXP_DISCARD)
> - return p;
> -
> + subevalvar(p, var, 0, subtype, startloc, varflags,
> + flag & ~QUOTES_ESC);
> varflags &= ~VSNUL;
> + /*
> + * Remove any recorded regions beyond
> + * start of variable
> + */
> + removerecordregions(startloc);
> goto again;
> }
>
> @@ -744,14 +759,20 @@ again:
> varunset(p, var, 0, 0);
>
> if (subtype == VSLENGTH) {
> - if (flag & EXP_DISCARD)
> - return p;
> - cvtnum(varlen > 0 ? varlen : 0, flag);
> + cvtnum(varlen > 0 ? varlen : 0);
> goto record;
> }
>
> - if (subtype == VSNORMAL)
> - goto record;
> + if (subtype == VSNORMAL) {
> +record:
> + if (quoted) {
> + quoted = *var == '@' && shellparam.nparam;
> + if (!quoted)
> + goto end;
> + }
> + recordregion(startloc, expdest - (char *)stackblock(), quoted);
> + goto end;
> + }
>
> #ifdef DEBUG
> switch (subtype) {
> @@ -765,28 +786,45 @@ again:
> }
> #endif
>
> - flag |= varlen < 0 ? EXP_DISCARD : 0;
> - if (!(flag & EXP_DISCARD)) {
> + if (varlen >= 0) {
> /*
> * Terminate the string and start recording the pattern
> * right after it
> */
> STPUTC('\0', expdest);
> + patloc = expdest - (char *)stackblock();
> + if (subevalvar(p, NULL, patloc, subtype,
> + startloc, varflags, flag) == 0) {
> + int amount = expdest - (
> + (char *)stackblock() + patloc - 1
> + );
> + STADJUST(-amount, expdest);
> + }
> + /* Remove any recorded regions beyond start of variable */
> + removerecordregions(startloc);
> + goto record;
> }
>
> - patloc = expdest - (char *)stackblock();
> - p = subevalvar(p, NULL, patloc, startloc, varflags, flag);
> -
> -record:
> - if (flag & EXP_DISCARD)
> - return p;
> + varlen = 0;
>
> - if (quoted) {
> - quoted = *var == '@' && shellparam.nparam;
> - if (!quoted)
> - return p;
> +end:
> + if (subtype != VSNORMAL) { /* skip to end of alternative */
> + int nesting = 1;
> + for (;;) {
> + if ((c = (signed char)*p++) == CTLESC)
> + p++;
> + else if (c == CTLBACKQ) {
> + if (varlen >= 0)
> + argbackq = argbackq->next;
> + } else if (c == CTLVAR) {
> + if ((*p++ & VSTYPE) != VSNORMAL)
> + nesting++;
> + } else if (c == CTLENDVAR) {
> + if (--nesting == 0)
> + break;
> + }
> + }
> }
> - recordregion(startloc, expdest - (char *)stackblock(), quoted);
> return p;
> }
>
> @@ -795,17 +833,15 @@ record:
> * Put a string on the stack.
> */
>
> -static size_t memtodest(const char *p, size_t len, int flags)
> +static void memtodest(const char *p, size_t len, int flags)
> {
> const char *syntax = flags & EXP_QUOTED ? DQSYNTAX : BASESYNTAX;
> char *q;
> - char *s;
>
> if (unlikely(!len))
> - return 0;
> + return;
>
> q = makestrspace(len * 2, expdest);
> - s = q;
>
> do {
> int c = (signed char)*p++;
> @@ -820,7 +856,6 @@ static size_t memtodest(const char *p, size_t len,
> int flags)
> } while (--len);
>
> expdest = q;
> - return q - s;
> }
>
>
> @@ -847,18 +882,10 @@ varvalue(char *name, int varflags, int flags, int quoted)
> char sepc;
> char **ap;
> int subtype = varflags & VSTYPE;
> - int discard = (subtype == VSPLUS || subtype == VSLENGTH) |
> - (flags & EXP_DISCARD);
> + int discard = subtype == VSPLUS || subtype == VSLENGTH;
> ssize_t len = 0;
> char c;
>
> - if (!subtype) {
> - if (discard)
> - return -1;
> -
> - sh_error("Bad substitution");
> - }
> -
> flags |= EXP_KEEPNUL;
> flags &= discard ? ~QUOTES_ESC : ~0;
> sep = (flags & EXP_FULL) << CHAR_BIT;
> @@ -878,7 +905,7 @@ varvalue(char *name, int varflags, int flags, int quoted)
> if (num == 0)
> return -1;
> numvar:
> - len = cvtnum(num, flags);
> + len = cvtnum(num);
> break;
> case '-':
> p = makestrspace(NOPTS, expdest);
> @@ -952,7 +979,6 @@ value:
>
> if (discard)
> STADJUST(-len, expdest);
> -
> return len;
> }
>
> @@ -1704,6 +1730,7 @@ casematch(union node *pattern, char *val)
> argbackq = pattern->narg.backquote;
> STARTSTACKSTR(expdest);
> argstr(pattern->narg.text, EXP_TILDE | EXP_CASE);
> + STACKSTRNUL(expdest);
> ifsfree();
> result = patmatch(stackblock(), val);
> popstackmark(&smark);
> @@ -1714,13 +1741,15 @@ casematch(union node *pattern, char *val)
> * Our own itoa().
> */
>
> -static size_t cvtnum(intmax_t num, int flags)
> +STATIC int
> +cvtnum(intmax_t num)
> {
> int len = max_int_length(sizeof(num));
> - char buf[len];
>
> - len = fmtstr(buf, len, "%" PRIdMAX, num);
> - return memtodest(buf, len, flags);
> + expdest = makestrspace(len, expdest);
> + len = fmtstr(expdest, len, "%" PRIdMAX, num);
> + STADJUST(len, expdest);
> + return len;
> }
>
> STATIC void
> diff --git a/src/expand.h b/src/expand.h
> index c44b848..617b851 100644
> --- a/src/expand.h
> +++ b/src/expand.h
> @@ -59,11 +59,11 @@ struct arglist {
> #define EXP_WORD 0x80 /* expand word in parameter expansion */
> #define EXP_QUOTED 0x100 /* expand word in double quotes */
> #define EXP_KEEPNUL 0x200 /* do not skip NUL characters */
> -#define EXP_DISCARD 0x400 /* discard result of expansion */
>
>
> union node;
> void expandarg(union node *, struct arglist *, int);
> +void expari(int);
> #define rmescapes(p) _rmescapes((p), 0)
> char *_rmescapes(char *, int);
> int casematch(union node *, char *);
> --
> 2.19.1
Re: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
- Subject: Re: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
- From: Michael Greenberg <Michael.Greenberg@xxxxxxxxxx>
- Date: Thu, 15 Nov 2018 16:53:44 +0000
- Accept-language: en-US
- In-reply-to: <CAEtFKstthNzvFreHuPe38MoV9ZpnWau+ctNv4jMu4NM0Odmnxg@mail.gmail.com>
- References: <CAEtFKstthNzvFreHuPe38MoV9ZpnWau+ctNv4jMu4NM0Odmnxg@mail.gmail.com>
- References:
- [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
- From: Devin Hussey
- [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
- Prev by Date: Re: [PATCH] Make mktokens accept a random TMPDIR, replace `...` with $(...).
- Next by Date: Re: [PATCH] Use a real non-cryptographic hash algorithm
- Previous by thread: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
- Next by thread: Re: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
- Index(es):
 |