NetBSD Problem Report #52547

From www@NetBSD.org  Sun Sep 17 05:29:44 2017
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 7B52E7A1F6
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 17 Sep 2017 05:29:44 +0000 (UTC)
Message-Id: <20170917052943.4FBCF7A2A1@mollari.NetBSD.org>
Date: Sun, 17 Sep 2017 05:29:43 +0000 (UTC)
From: yan12125@gmail.com
Reply-To: yan12125@gmail.com
To: gnats-bugs@NetBSD.org
Subject: history_get_history_state() can report wrong history length
X-Send-Pr-Version: www-1.0

>Number:         52547
>Category:       lib
>Synopsis:       history_get_history_state() can report wrong history length
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 17 05:30:00 +0000 2017
>Closed-Date:    Sun Sep 17 17:33:12 +0000 2017
>Last-Modified:  Sun Sep 17 17:40:00 +0000 2017
>Originator:     Yen Chi Hsuan
>Release:        8.99.2
>Organization:
>Environment:
NetBSD  8.99.2 NetBSD 8.99.2 (GENERIC) #0: Sat Sep 16 09:28:38 UTC 2017  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/amd64/compile/GENERIC amd64
>Description:
With NetBSD's libedit, history_get_history_state() can report wrong history length if called immediately after read_history(). The reason is that read_history() does not update history_length [1].

[1] https://github.com/NetBSD/src/blob/trunk/lib/libedit/readline.c#L1350
>How-To-Repeat:
Create readline_test.c:

#include <readline/history.h>

int main()
{
        read_history("readline_history.txt");
        HISTORY_STATE *state = history_get_history_state();
        printf("%d\n", state->length);
        return 0;
}

And readline_history.txt:

_HiStOrY_V2_
1+1
1+2

Compiling it:

$ gcc readline_test.c -ledit

The result is 0:

$ ./a.out
0

Which should be 2.
>Fix:
Apple has a patch for readline.c with correct history_length handling [1]. Indeed the same test program gives correct results on macOS with Apple's forked version libedit-48.50.1.

int
read_history(const char *filename)
{
	HistEvent ev;

	if (h == NULL || e == NULL)
		rl_initialize();
	if (filename == NULL && (filename = _default_history_file()) == NULL)
		return errno;
	if (history(h, &ev, H_LOAD, filename) == -1)
		return (errno ? errno : EINVAL);
	if (history(h, &ev, H_GETSIZE) == 0)
		history_length = ev.num;

	return (!(history_length > 0)); /* return 0 if all is okay */
}

[1] https://opensource.apple.com/source/libedit/libedit-48.50.1/src/readline.c.auto.html

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: kre@NetBSD.org
State-Changed-When: Sun, 17 Sep 2017 08:11:44 +0000
State-Changed-Why:
I have incorporated a (slightly modified version of) the code that
you sent, it seems to work for me using the test program you sent.

Can you please update, test, and verify that the fix seems correct?


From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52547 CVS commit: src/lib/libedit
Date: Sun, 17 Sep 2017 08:10:08 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Sun Sep 17 08:10:08 UTC 2017

 Modified Files:
 	src/lib/libedit: readline.c

 Log Message:
 PR lib/52547 - read_history (readline.c) should now sets history_length.

 Patch from Yen Chi Hsuan in the PR, extracted from Apple's version of
 readline.c, then modified by me to be consistent about what the return
 value really is.


 To generate a diff of this commit:
 cvs rdiff -u -r1.143 -r1.144 src/lib/libedit/readline.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Chi-Hsuan Yen <yan12125@gmail.com>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org, netbsd-bugs@netbsd.org, gnats-admin@netbsd.org, 
	kre@netbsd.org
Subject: Re: lib/52547 (history_get_history_state() can report wrong history length)
Date: Sun, 17 Sep 2017 23:41:29 +0800

 On Sun, Sep 17, 2017 at 4:11 PM,  <kre@netbsd.org> wrote:
 > Synopsis: history_get_history_state() can report wrong history length
 >
 > State-Changed-From-To: open->feedback
 > State-Changed-By: kre@NetBSD.org
 > State-Changed-When: Sun, 17 Sep 2017 08:11:44 +0000
 > State-Changed-Why:
 > I have incorporated a (slightly modified version of) the code that
 > you sent, it seems to work for me using the test program you sent.
 >
 > Can you please update, test, and verify that the fix seems correct?
 >
 >
 >

 Thanks for the patch! I rebuild libedit and it works fine.

 Best,

 Yen Chi Hsuan

State-Changed-From-To: feedback->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Sun, 17 Sep 2017 17:33:12 +0000
State-Changed-Why:
Submitter confirms that problem is fixed


From: Robert Elz <kre@munnari.OZ.AU>
To: Chi-Hsuan Yen <yan12125@gmail.com>
Cc: gnats-bugs@netbsd.org, lib-bug-people@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/52547 (history_get_history_state() can report wrong history length)
Date: Mon, 18 Sep 2017 00:35:48 +0700

     Date:        Sun, 17 Sep 2017 23:41:29 +0800
     From:        Chi-Hsuan Yen <yan12125@gmail.com>
     Message-ID:  <CAMNjDR3oM6RBU8V_OV15isR264OnjNe6GNBp1d7hBad0Te17Xg@mail.gmail.com>

   | Thanks for the patch! I rebuild libedit and it works fine.

 Great, thanks for reporting the problem, and for finding the solution.

 kre

>Unformatted:

Home	PR Database Search