From Ingo Schwarze:
In libedit, the only way how H_ENTER can fail is memory exhaustion, too, and of course it is handled gracefully, returning -1 from history(). So of course, we will continue to handle it gracefully in add_history() as well, but we are free to decide what to do with the library state in this case because GNU just dies... I think the most reasonable course of action is to simply not change the library state in any way when add_history() fails due to memory exhaustion, but just return. If H_ENTER does not fail, we know that the history now contains at least one entry, so there is no need any longer to check the H_GETSIZE return value. And we can of course always set current_history_valid. While testing these changes, i noticed three problems so closely related that i'd like to fix them in the same diff. 1. libedit has the wrong prototype for add_history(). GNU readline-6.3 defines it as void add_history(const char *). Of course, that is very stupid - no way to report problems to the caller! But the whole point of a compatibility mode is being compatible, so we should ultimately change this. Of course, changing the prototype of a public symbol requires a libedit major bump. I don't want to do that casually. Rather, i will take a note and change the prototype the next time we need a libedit major bump for more important reasons. For now, let's just always return 0. 2. While *implicitely* pushing an old entry off the history increments history_base in GNU readline, testing reveals that *explicitly* deleting one does not. Again, this is not documented, but it applies to both remove_history() and stifle_history(). So delete history_base manipulation from stifle_history(), which also allows to simplify the code and delete two automatic variables. 3. GNU readline add_history(NULL) crashes with a segfault. There is nothing wrong with having a public interface behave that way. Many standard interfaces do, including strlen(3). Such crashes can even be useful to catch buggy application programs. In libedit/readline.c rev. 1.104, Christos made add_history() silently ignore this coding error, according to the commit message to hide a bug in nslookup(1). That change was never merged to OpenBSD. I strongly disagree with this change. If nslookup(1) is still broken, that program needs to be fixed instead. In any case, delete the bogus check; hiding bugs is dangerous.
This commit is contained in:
parent
162319ac6a
commit
956883ecd7
|
@ -1,4 +1,4 @@
|
|||
/* $NetBSD: readline.c,v 1.134 2016/05/31 19:25:17 christos Exp $ */
|
||||
/* $NetBSD: readline.c,v 1.135 2016/06/02 15:11:18 christos Exp $ */
|
||||
|
||||
/*-
|
||||
* Copyright (c) 1997 The NetBSD Foundation, Inc.
|
||||
|
@ -31,7 +31,7 @@
|
|||
|
||||
#include "config.h"
|
||||
#if !defined(lint) && !defined(SCCSID)
|
||||
__RCSID("$NetBSD: readline.c,v 1.134 2016/05/31 19:25:17 christos Exp $");
|
||||
__RCSID("$NetBSD: readline.c,v 1.135 2016/06/02 15:11:18 christos Exp $");
|
||||
#endif /* not lint && not SCCSID */
|
||||
|
||||
#include <sys/types.h>
|
||||
|
@ -1179,17 +1179,13 @@ stifle_history(int max)
|
|||
{
|
||||
HistEvent ev;
|
||||
HIST_ENTRY *he;
|
||||
int i, len;
|
||||
|
||||
if (h == NULL || e == NULL)
|
||||
rl_initialize();
|
||||
|
||||
len = history_length;
|
||||
if (history(h, &ev, H_SETSIZE, max) == 0) {
|
||||
max_input_history = max;
|
||||
if (max < len)
|
||||
history_base += len - max;
|
||||
for (i = 0; i < len - max; i++) {
|
||||
while (history_length > max) {
|
||||
he = remove_history(0);
|
||||
el_free(he->data);
|
||||
el_free((void *)(unsigned long)he->line);
|
||||
|
@ -1452,18 +1448,19 @@ add_history(const char *line)
|
|||
{
|
||||
HistEvent ev;
|
||||
|
||||
if (line == NULL)
|
||||
return 0;
|
||||
|
||||
if (h == NULL || e == NULL)
|
||||
rl_initialize();
|
||||
|
||||
(void)history(h, &ev, H_ENTER, line);
|
||||
if (history(h, &ev, H_GETSIZE) == 0)
|
||||
if (history(h, &ev, H_ENTER, line) == -1)
|
||||
return 0;
|
||||
|
||||
(void)history(h, &ev, H_GETSIZE);
|
||||
if (ev.num == history_length)
|
||||
history_base++;
|
||||
else
|
||||
history_length = ev.num;
|
||||
current_history_valid = 1;
|
||||
|
||||
return !(history_length > 0); /* return 0 if all is okay */
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
|
|
Loading…
Reference in New Issue