Module Name: src
Committed By: christos
Date: Thu Jun 2 15:11:18 UTC 2016
Modified Files:
src/lib/libedit: readline.c
Log Message:
>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.
To generate a diff of this commit:
cvs rdiff -u -r1.134 -r1.135 src/lib/libedit/readline.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/lib/libedit/readline.c
diff -u src/lib/libedit/readline.c:1.134 src/lib/libedit/readline.c:1.135
--- src/lib/libedit/readline.c:1.134 Tue May 31 15:25:17 2016
+++ src/lib/libedit/readline.c Thu Jun 2 11:11:18 2016
@@ -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;
}
