On Mon, 28 Aug 2017 11:13:03 +0200, Jeremie Courreges-Anglas wrote: > 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().
I agree. OK millert@ - todd
