[src/trunk]: src/lib/libedit From Ingo Schwarze:

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/0ed21605bfe4
branches:  trunk
changeset: 345626:0ed21605bfe4
user:      christos <christos%NetBSD.org@localhost>
date:      Thu Jun 02 15:11:18 2016 +0000

description:
>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.

diffstat:

 lib/libedit/readline.c |  25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

diffs (64 lines):

diff -r d137c132fb63 -r 0ed21605bfe4 lib/libedit/readline.c
--- a/lib/libedit/readline.c    Thu Jun 02 12:28:11 2016 +0000
+++ b/lib/libedit/readline.c    Thu Jun 02 15:11:18 2016 +0000
@@ -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 @@
 {
        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 @@
 {
        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;
 }