On Sun, Jul 08, 2018 at 02:54:32PM +0200, Jeremie Courreges-Anglas wrote:
> On Sun, Jul 08 2018, Anton Lindqvist <[email protected]> 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?
>
> s/has/have/ in the comment you're adding, I think, except for this
yep
> ok jca@ for the diff and regress parts
ok tb for these as well
>
> > 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;
> > }
> >
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
