On Sun, Jul 08, 2018 at 05:18:28PM +0900, Anton Lindqvist wrote:
> On Sat, Jul 07, 2018 at 10:06:41AM +0200, Andreas Kusalananda Kähäri wrote:
> > Hi,
> >
> > I'm not sure this is a bug or a misunderstanding by me, but it sure
> > surprised me:
> >
> > $ set -- file.txt txt
> >
> > $ printf 'Two values: %s %s\n' "$1" "$2"
> > Two values: file.txt txt
> >
> > $ printf 'First value with suffix removed: %s\n' "${1%.*}"
> > First value with suffix removed: file
> >
> > $ printf 'First value with second value removed from end: %s\n'
> > "${1%.$2}"
> > First value with second value removed from end: txt
> >
> > I would have expected the last two parameter expansion to generate the
> > same result ("file"). Also, removing the dot ("${1%$2}") generates an
> > empty result.
> >
> > Note that using variables, this works as expected:
> >
> > $ filename=file.txt
> > $ suffix=txt
> > $ printf 'First value with second value removed from end: %s\n'
> > "${filename%.$suffix}"
> > First value with second value removed from end: file
> >
> > This is on a current system using either /bin/sh or /bin/ksh and the
> > effect of trying to remove from the start of the string is similar.
>
> That's indeed a bug. Lookup of positional arguments ($1 and $2) is done
> using global() which in turn returns a pointer to static storage for
> read-only variables (positional arguments are read-only). So the second
> call to global() overwrites the previous one. Here's a first stab in
> which read-only variables are copied during expansion. I ended up adding
> a new function in order to avoid deep indenting. Also, zero st_head in
> order to simplify the free loop; otherwise st_head.var would contain
> stack garbage.
>
> Comments? OK?
A fix has now been committed, thanks for the report!
>
> Index: eval.c
> ===================================================================
> RCS file: /cvs/src/bin/ksh/eval.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 eval.c
> --- eval.c 9 Apr 2018 17:53:36 -0000 1.60
> +++ eval.c 8 Jul 2018 08:17:05 -0000
> @@ -58,6 +58,8 @@ static char *tilde(char *);
> static char *homedir(char *);
> static void alt_expand(XPtrV *, char *, char *, char *, int);
>
> +static struct tbl *varcpy(struct tbl *);
> +
> /* compile and expand word */
> char *
> substitute(const char *cp, int f)
> @@ -190,7 +192,8 @@ expand(char *cp, /* input word */
> doblank = 0;
> make_magic = 0;
> word = (f&DOBLANK) ? IFS_WS : IFS_WORD;
> - st_head.next = NULL;
> +
> + memset(&st_head, 0, sizeof(st_head));
> st = &st_head;
>
> while (1) {
> @@ -305,7 +308,7 @@ expand(char *cp, /* input word */
> st->stype = stype;
> st->base = Xsavepos(ds, dp);
> st->f = f;
> - st->var = x.var;
> + st->var = varcpy(x.var);
> st->quote = quote;
> /* skip qualifier(s) */
> if (stype)
> @@ -577,7 +580,7 @@ expand(char *cp, /* input word */
> Xinit(ds, dp, 128, ATEMP);
> }
> if (c == 0)
> - return;
> + goto done;
> if (word != IFS_NWS)
> word = ctype(c, C_IFSWS) ? IFS_WS : IFS_NWS;
> } else {
> @@ -682,6 +685,14 @@ expand(char *cp, /* input word */
> word = IFS_WORD;
> }
> }
> +
> +done:
> + for (st = &st_head; st != NULL; st = st->next) {
> + if (st->var == NULL || (st->var->flag & RDONLY) == 0)
> + continue;
> +
> + afree(st->var, ATEMP);
> + }
> }
>
> /*
> @@ -1285,4 +1296,24 @@ alt_expand(XPtrV *wp, char *start, char
> }
> }
> return;
> +}
> +
> +/*
> + * Copy the given variable if it's flagged as read-only.
> + * Such variables has static storage and only one can therefore be
> referenced at
> + * a time.
> + * This is necessary in order to allow variable expansion expressions to
> refer
> + * to multiple read-only variables.
> + */
> +static struct tbl *
> +varcpy(struct tbl *vp)
> +{
> + struct tbl *cpy;
> +
> + if ((vp->flag & RDONLY) == 0)
> + return vp;
> +
> + cpy = alloc(sizeof(struct tbl), ATEMP);
> + memcpy(cpy, vp, sizeof(struct tbl));
> + return cpy;
> }
