Right now the code uses two methods to access "valid" entries in the
history array. One method only looks at entries in [history;histptr],
the other method walks the array until it finds a NULL pointer.
The latter method appears fragile to me. *IIUC* the test at the
beginning of history_write() means that history_write() only walks the
array when the latter is full ie when
"histptr == &history[histsize -1]", so it doesn't access dangling
pointers after "histptr"; also the "if (cmd == NULL)" test looks like
dead code.
I find this error-prone and think that we should rely on a single method
to walk the array. This is what the third hunk does. The first &
second hunks undo part of my previous commit, leaving freed entries
alone, so that we don't send the wrong hint & encourage code such as the
current history_write().
Thoughts? ok?
Index: history.c
===================================================================
RCS file: /d/cvs/src/bin/ksh/history.c,v
retrieving revision 1.66
diff -u -p -p -u -r1.66 history.c
--- history.c 27 Aug 2017 17:10:32 -0000 1.66
+++ history.c 28 Aug 2017 09:06:36 -0000
@@ -439,10 +439,8 @@ histreset(void)
{
char **hp;
- for (hp = history; hp <= histptr; hp++) {
+ for (hp = history; hp <= histptr; hp++)
afree(*hp, APERM);
- *hp = NULL;
- }
histptr = history - 1;
hist_source->line = 0;
@@ -530,10 +528,8 @@ sethistsize(int n)
char **hp;
offset = n - 1;
- for (hp = history; hp < histptr - offset; hp++) {
+ for (hp = history; hp < histptr - offset; hp++)
afree(*hp, APERM);
- *hp = NULL;
- }
memmove(history, histptr - offset, n * sizeof(char *));
}
@@ -771,8 +767,7 @@ hist_init(Source *s)
static void
history_write(void)
{
- char *cmd, *encoded;
- int i;
+ char **hp, *encoded;
/* see if file has grown over 25% */
if (line_co < histsize + (histsize / 4))
@@ -784,12 +779,8 @@ history_write(void)
bi_errorf("failed to rewrite history file - %s",
strerror(errno));
}
- for (i = 0; i < histsize; i++) {
- cmd = history[i];
- if (cmd == NULL)
- break;
-
- if (stravis(&encoded, cmd, VIS_SAFE | VIS_NL) != -1) {
+ for (hp = history; hp <= histptr; hp++) {
+ if (stravis(&encoded, *hp, VIS_SAFE | VIS_NL) != -1) {
if (fprintf(histfh, "%s\n", encoded) == -1) {
free(encoded);
return;
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
