Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/bin/sh Many internal memory management type fixes.
details: https://anonhg.NetBSD.org/src/rev/6b685df34508
branches: trunk
changeset: 824752:6b685df34508
user: kre <kre%NetBSD.org@localhost>
date: Sat Jun 17 07:22:12 2017 +0000
description:
Many internal memory management type fixes.
PR bin/52302 (core dump with interactive shell, here doc and error
on same line) is fixed. (An old bug.)
echo "$( echo x; for a in $( seq 1000 ); do printf '%s\n'; done; echo y )"
consistently prints 1002 lines (x, 1000 empty ones, then y) as it should
(And you don't want to know what it did before, or why.) (Another old one.)
(Recently added) Problems with ~ expansion fixed (mem management related).
Proper fix for the cwrappers configure problem (which includes the quick
fix that was done earlier, but extends upon that to be correct). (This was
another newly added problem.)
And the really devious (and rare) old bug - if STACKSTRNUL() needs to
allocate a new buffer in which to store the \0, calculate the size of
the string space remaining correctly, unlike when SPUTC() grows the
buffer, there is no actual data being stored in the STACKSTRNUL()
case - the string space remaining was calculated as one byte too few.
That would be harmless, unless the next buffer also filled, in which
case it was assumed that it was really full, not one byte less, meaning
one junk char (a nul, or anything) was being copied into the next (even
bigger buffer) corrupting the data.
Consistent use of stalloc() to allocate a new block of (stack) memory,
and grabstackstr() to claim a block of (stack) memory that had already
been occupied but not claimed as in use. Since grabstackstr is implemented
as just a call to stalloc() this is a no-op change in practice, but makes
it much easier to comprehend what is really happening. Previous code
sometimes used stalloc() when the use case was really for grabstackstr().
Change grabstackstr() to actually use the arg passed to it, instead of
(not much better than) guessing how much space to claim,
More care when using unstalloc()/ungrabstackstr() to return space, and in
particular when the stack must be returned to its previous state, rather than
just returning no-longer needed space, neither of those work. They also don't
work properly if there have been (really, even might have been) any stack mem
allocations since the last stalloc()/grabstackstr(). (If we know there
cannot have been then the alloc/release sequence is kind of pointless.)
To work correctly in general we must use setstackmark()/popstackmark() so
do that when needed. Have those also save/restore the top of stack string
space remaining.
[Aside: for those reading this, the "stack" mentioned is not
in any way related to the thing used for maintaining the C
function call state, ie: the "stack segment" of the program,
but the shell's internal memory management strategy.]
More comments to better explain what is happening in some cases.
Also cleaned up some hopelessly broken DEBUG mode data that were
recently added (no effect on anyone but the poor semi-human attempting
to make sense of it...).
User visible changes:
Proper counting of line numbers when a here document is delimited
by a multi-line end-delimiter, as in
cat << 'REALLY
END'
here doc line 1
here doc line 2
REALLY
END
(which is an obscure case, but nothing says should not work.) The \n
in the end-delimiter of the here doc (the last one) was not incrementing
the line number, which from that point on in the script would be 1 too
low (or more, for end-delimiters with more than one \n in them.)
With tilde expansion:
unset HOME; echo ~
changed to return getpwuid(getuid())->pw_home instead of failing (returning ~)
POSIX says this is unspecified, which makes it difficult for a script to
compensate for being run without HOME set (as in env -i sh script), so
while not able to be used portably, this seems like a useful extension
(and is implemented the same way by some other shells).
Further, with
HOME=; printf %s ~
we now write nothing (which is required by POSIX - which requires ~ to
expand to the value of $HOME if it is set) previously if $HOME (in this
case) or a user's directory in the passwd file (for ~user) were a null
STRING, We failed the ~ expansion and left behind '~' or '~user'.
diffstat:
bin/sh/exec.c | 18 ++-
bin/sh/expand.c | 291 ++++++++++++++++++++++++++++++++---------------------
bin/sh/memalloc.c | 23 +++-
bin/sh/memalloc.h | 8 +-
bin/sh/parser.c | 78 ++++++++++----
5 files changed, 267 insertions(+), 151 deletions(-)
diffs (truncated from 927 to 300 lines):
diff -r 711ffb243606 -r 6b685df34508 bin/sh/exec.c
--- a/bin/sh/exec.c Sat Jun 17 04:19:12 2017 +0000
+++ b/bin/sh/exec.c Sat Jun 17 07:22:12 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: exec.c,v 1.49 2017/06/07 05:08:32 kre Exp $ */
+/* $NetBSD: exec.c,v 1.50 2017/06/17 07:22:12 kre Exp $ */
/*-
* Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
#if 0
static char sccsid[] = "@(#)exec.c 8.4 (Berkeley) 6/8/95";
#else
-__RCSID("$NetBSD: exec.c,v 1.49 2017/06/07 05:08:32 kre Exp $");
+__RCSID("$NetBSD: exec.c,v 1.50 2017/06/17 07:22:12 kre Exp $");
#endif
#endif /* not lint */
@@ -334,11 +334,10 @@
*path = p + 1;
else
*path = NULL;
- return stalloc(len);
+ return grabstackstr(q + strlen(name) + 1);
}
-
/*** Command hashing code ***/
@@ -617,14 +616,17 @@
if (!S_ISREG(statb.st_mode))
goto loop;
if (pathopt) { /* this is a %func directory */
+ char *endname;
+
if (act & DO_NOFUNC)
goto loop;
- stalloc(strlen(fullname) + 1);
+ endname = fullname + strlen(fullname) + 1;
+ grabstackstr(endname);
readcmdfile(fullname);
if ((cmdp = cmdlookup(name, 0)) == NULL ||
cmdp->cmdtype != CMDFUNCTION)
error("%s not defined in %s", name, fullname);
- stunalloc(fullname);
+ ungrabstackstr(fullname, endname);
goto success;
}
#ifdef notdef
@@ -644,7 +646,11 @@
TRACE(("searchexec \"%s\" returns \"%s\"\n", name, fullname));
INTOFF;
if (act & DO_ALTPATH) {
+ /*
+ * this should be a grabstackstr() but is not needed:
+ * fullname is no longer needed for anything
stalloc(strlen(fullname) + 1);
+ */
cmdp = &loc_cmd;
} else
cmdp = cmdlookup(name, 1);
diff -r 711ffb243606 -r 6b685df34508 bin/sh/expand.c
--- a/bin/sh/expand.c Sat Jun 17 04:19:12 2017 +0000
+++ b/bin/sh/expand.c Sat Jun 17 07:22:12 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: expand.c,v 1.115 2017/06/07 09:31:30 kre Exp $ */
+/* $NetBSD: expand.c,v 1.116 2017/06/17 07:22:12 kre Exp $ */
/*-
* Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
#if 0
static char sccsid[] = "@(#)expand.c 8.5 (Berkeley) 5/15/95";
#else
-__RCSID("$NetBSD: expand.c,v 1.115 2017/06/07 09:31:30 kre Exp $");
+__RCSID("$NetBSD: expand.c,v 1.116 2017/06/17 07:22:12 kre Exp $");
#endif
#endif /* not lint */
@@ -225,9 +225,6 @@
int firsteq = 1;
const char *ifs = NULL;
int ifs_split = EXP_IFS_SPLIT;
-#ifdef DEBUG
- const char *ed = expdest;
-#endif
if (flag & EXP_IFS_SPLIT)
ifs = ifsval();
@@ -239,16 +236,16 @@
for (;;) {
switch (c = *p++) {
case '\0':
+ STACKSTRNUL(expdest);
VTRACE(DBG_EXPAND, ("argstr returning at \"\" "
- "added \"%.*s\" to expdest (followed by: %2.2x)\n",
- expdest-ed, ed, *expdest));
+ "added \"%s\" to expdest\n", stackblock()));
return p - 1;
case CTLENDVAR: /* end of expanding yyy in ${xxx-yyy} */
case CTLENDARI: /* end of a $(( )) string */
+ STACKSTRNUL(expdest);
VTRACE(DBG_EXPAND, ("argstr returning at \"%.6s\"..."
- " after %2.2x; "
- "added \"%.*s\" to expdest (followed by: %2.2x)\n",
- p, p[-1]&0xff, expdest-ed, ed, *expdest));
+ " after %2.2X; added \"%s\" to expdest\n",
+ p, (c&0xff), stackblock()));
return p;
case CTLQUOTEMARK:
/* "$@" syntax adherence hack */
@@ -272,27 +269,43 @@
STPUTC(c, expdest);
c = *p++;
STPUTC(c, expdest);
+ if (c == '\n') /* should not happen, but ... */
+ line_number++;
break;
- case CTLVAR:
+ case CTLVAR: {
+#ifdef DEBUG
+ unsigned int pos = expdest - stackblock();
+#endif
p = evalvar(p, (flag & ~EXP_IFS_SPLIT) | (flag & ifs_split));
+ STACKSTRNUL(expdest);
VTRACE(DBG_EXPAND, ("argstr evalvar "
- "added \"%.*s\" to expdest (followed by: %2.2x)\n",
- expdest-ed, ed, *expdest));
+ "added \"%s\" to expdest\n",
+ stackblock() + pos));
break;
+ }
case CTLBACKQ:
- case CTLBACKQ|CTLQUOTE:
+ case CTLBACKQ|CTLQUOTE: {
+#ifdef DEBUG
+ unsigned int pos = expdest - stackblock();
+#endif
expbackq(argbackq->n, c & CTLQUOTE, flag);
argbackq = argbackq->next;
- VTRACE(DBG_EXPAND, ("argstr expbackq "
- "added \"%.*s\" to expdest (followed by: %2.2x)\n",
- expdest-ed, ed, *expdest));
+ STACKSTRNUL(expdest);
+ VTRACE(DBG_EXPAND, ("argstr expbackq added \"%s\" "
+ "to expdest\n", stackblock() + pos));
break;
- case CTLARI:
+ }
+ case CTLARI: {
+#ifdef DEBUG
+ unsigned int pos = expdest - stackblock();
+#endif
p = expari(p);
+ STACKSTRNUL(expdest);
VTRACE(DBG_EXPAND, ("argstr expari "
- "+ \"%.*s\" to dest (fwd by: %2.2x) p=\"%.5s...\"\n",
- expdest-ed, ed, *expdest, p));
+ "+ \"%s\" to expdest p=\"%.5s...\"\n",
+ stackblock() + pos, p));
break;
+ }
case ':':
case '=':
/*
@@ -333,20 +346,29 @@
const char *home;
int quotes = flag & (EXP_GLOB | EXP_CASE);
char *user;
+ struct stackmark smark;
+#ifdef DEBUG
+ unsigned int offs = expdest - stackblock();
+#endif
- user = expdest; /* we will just borrow top of stack */
+ setstackmark(&smark);
+ user = stackblock(); /* we will just borrow top of stack */
+
while ((c = *++p) != '\0') {
switch(c) {
- case CTLESC:
- case CTLVAR:
- case CTLBACKQ:
+ case CTLESC: /* any of these occurring */
+ case CTLVAR: /* means ~ expansion */
+ case CTLBACKQ: /* does not happen at all */
case CTLBACKQ | CTLQUOTE:
- case CTLARI:
+ case CTLARI: /* just leave original unchanged */
case CTLENDARI:
case CTLQUOTEMARK:
+ case CTLENDVAR:
+ case '\n':
+ popstackmark(&smark);
return (startp);
case CTLNONL:
- break;
+ continue;
case ':':
if (flag & EXP_VARTILDE)
goto done;
@@ -358,24 +380,46 @@
}
done:
STACKSTRNUL(user);
+ user = stackblock(); /* to start of collected username */
- CTRACE(DBG_EXPAND, ("exptilde, found \"%s\" :", expdest));
- if (*expdest == '\0')
+ CTRACE(DBG_EXPAND, ("exptilde, found \"~%s\"", user));
+ if (*user == '\0') {
home = lookupvar("HOME");
- else if ((pw = getpwnam(expdest)) == NULL)
+ /*
+ * if HOME is unset, results are unspecified...
+ * we used to just leave the ~ unchanged, but
+ * (some) other shells do ... and this seems more useful.
+ */
+ if (home == NULL && (pw = getpwuid(getuid())) != NULL)
+ home = pw->pw_dir;
+ } else if ((pw = getpwnam(user)) == NULL) {
+ /*
+ * If user does not exist, results are undefined.
+ * so we can abort() here if we want, but let's not!
+ */
home = NULL;
- else
+ } else
home = pw->pw_dir;
- STUNSTR(user - expdest); /* return borrowed string space */
+
+ VTRACE(DBG_EXPAND, (" ->\"%s\"", home ? home : "<<NULL>>"));
+ popstackmark(&smark); /* now expdest is valid again */
- if (home == NULL || *home == '\0')
+ /*
+ * Posix XCU 2.6.1: The value of $HOME (for ~) or the initial
+ * working directory from getpwnam() for ~user
+ * Nothing there about "except if a null string". So do what it wants.
+ */
+ if (home == NULL /* || *home == '\0' */) {
+ CTRACE(DBG_EXPAND, (": returning unused \"%s\"\n", startp));
return startp;
- while ((c = *home++) != '\0') {
+ } while ((c = *home++) != '\0') {
if (quotes && SQSYNTAX[(int)c] == CCTL)
STPUTC(CTLESC, expdest);
STPUTC(c, expdest);
}
- CTRACE(DBG_EXPAND, ("returning \"%s\"\n", p));
+ CTRACE(DBG_EXPAND, (": added %d \"%.*s\" returning \"%s\"\n",
+ expdest - stackblock() - offs, expdest - stackblock() - offs,
+ stackblock() + offs, p));
return (p);
}
@@ -431,8 +475,13 @@
/*
- * Expand arithmetic expression. Backup to start of expression,
- * evaluate, place result in (backed up) result, adjust string position.
+ * Expand arithmetic expression.
+ *
+ * In this incarnation, we start at the beginning (yes, "Let's start at the
+ * very beginning. A very good place to start.") and collect the expression
+ * until the end - which means expanding anything contained within.
+ *
+ * Fortunately, argstr() just happens to do that for us...
*/
STATIC const char *
expari(const char *p)
@@ -442,24 +491,11 @@
int adjustment;
int begoff;
int quoted;
-#ifdef DEBUG
- const char *ed = expdest;
-#endif
+ struct stackmark smark;
/* ifsfree(); */
/*
- * This routine is slightly over-complicated for
- * efficiency. First we make sure there is
- * enough space for the result, which may be bigger
- * than the expression if we add exponentation. Next we
- * scan backwards looking for the start of arithmetic. If the
- * next previous character is a CTLESC character, then we
- * have to rescan starting from the beginning since CTLESC
- * characters have to be processed left to right.
- */
-
- /*
* SPACE_NEEDED is enough for all possible digits (rounded up)
* plus possible "-", and the terminating '\0', hence, plus 2
*
Home |
Main Index |
Thread Index |
Old Index