On Fri, Oct 18 2019, [email protected] wrote:
> This can be hard to debug, and could indeed prevent debugging, since
> it can take out the ability to run /bin/sh, depending on how you've
> set up your environment.
>
> Before:
> $ HISTSIZE=1000000000 /bin/sh
> /bin/sh: internal error: unable to allocate memory
> <crashes> <--- no ksh
Could you provide more details please? A fatal error and a crash are
very different problems. Does "no ksh" imply that the *current* shell
exited or crashed, leaving you with no shell at all?
Note that setting HISTSIZE like this affects your *current* shell and
prevents it to run /bin/sh. Use env to only affect the child shell:
env HISTSIZE=1000000000 ksh
ksh: internal error: unable to allocate memory
> With this pa[tch]
> $ HISTSIZE=1000000000 ./ksh
> ./ksh: internal error: unable to allocate 16+8000000008 bytes of memory
> Unable to set the history size to 1000000000
> hostname$ ^D <--- ksh running
Your diff doesn't apply as is. After applying it, the child ksh will
indeed keep running even if setting HISTSIZE fails. But it reliably
crashes on exit if HISTSIZE was set in its startup environment:
--8<--
ritchie /usr/src/bin/ksh$ env ENV=/dev/null HISTFILE=ksh_history HISTSIZE=1000
obj/ksh
ritchie$ ^D
Unable to set the history size to 0
Trace/BPT trap (core dumped)
-->8--
I think we can do ourselves a favor and not involve unwinding/setjmps
for this simple error case. Using alloc.c to allocate histbase is not
useful, so just switch to reallocarray. Allocation failure at startup
is still fatal because the code can't cope with a NULL history array.
ok?
Index: history.c
===================================================================
RCS file: /cvs/src/bin/ksh/history.c,v
retrieving revision 1.82
diff -u -p -r1.82 history.c
--- history.c 28 Jun 2019 13:34:59 -0000 1.82
+++ history.c 19 Oct 2019 15:29:04 -0000
@@ -554,6 +554,7 @@ void
sethistsize(int n)
{
if (n > 0 && (uint32_t)n != histsize) {
+ char **tmp;
int offset = histptr - history;
/* save most recent history */
@@ -566,10 +567,15 @@ sethistsize(int n)
memmove(history, histptr - offset, n * sizeof(char *));
}
- histsize = n;
- histbase = areallocarray(histbase, n + 1, sizeof(char *),
APERM);
- history = histbase + 1;
- histptr = history + offset;
+ tmp = reallocarray(histbase, n + 1, sizeof(char *));
+ if (tmp != NULL) {
+ histbase = tmp;
+ histsize = n;
+ history = histbase + 1;
+ histptr = history + offset;
+ } else
+ warningf(false, "resizing history storage: %s",
+ strerror(errno));
}
}
@@ -613,8 +619,10 @@ init_histvec(void)
* allocate one extra element so that histptr always
* lies within array bounds
*/
- histbase = areallocarray(NULL, histsize + 1, sizeof(char *),
- APERM);
+ histbase = reallocarray(NULL, histsize + 1, sizeof(char *));
+ if (histbase == NULL)
+ internal_errorf("allocating history storage: %s",
+ strerror(errno));
*histbase = NULL;
history = histbase + 1;
histptr = history - 1;
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
