[Patches] [ python-Patches-1093585 ] sanity check for readline remove/replace

[SourceForge.net]

Patches item #1093585, was opened at 2004-12-31 03:22
Message generated for change (Comment added) made by loewis
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1093585&group_id=5470

Category: Modules
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 5
Submitted By: DSM (dsm001)
Assigned to: Nobody/Anonymous (nobody)
Summary: sanity check for readline remove/replace

Initial Comment:
Fix bug 1086603 (segfault in readline) by checking for
negative input indices in remove_history_item and
replace_history_item; GNU code doesn't always return NULL.

----------------------------------------------------------------------

>Comment By: Martin v. Löwis (loewis)
Date: 2005-02-27 21:34

Message:
Logged In: YES 
user_id=21627

Thanks for the patch. Applied as

NEWS 1.1246 and 1.1193.2.22
readline.c 2.80 and 2.79.2.1


----------------------------------------------------------------------

Comment By: Michiel de Hoon (mdehoon)
Date: 2005-01-21 06:26

Message:
Logged In: YES 
user_id=488897

I think that both solutions would be good (certainly better
than segfaulting). I'll write a patch review to the
python-dev mailing list outlining the pros and cons, so
hopefully a developer will look at this patch (I don't have
CVS write access, so I can't apply the patch myself).

----------------------------------------------------------------------

Comment By: DSM (dsm001)
Date: 2005-01-21 03:57

Message:
Logged In: YES 
user_id=1175690

You could do it, but it'd be a larger change: e.g.
get_history_item should change as well.  The API isn't very
listish.

Right now get_history_item(-1) returns None, which I left
alone because I didn't want to change any previously-working
behaviour of the code, though I doubt many people are
depending on that..  This comes at the cost of introducing
an arguable inconsistency: get_history_item(-1) simply
returns None but remove_history_item(-1) raises an error.

I wasn't sure to what degree py-gnu-readline is supposed to
hew to gnu-readline, so I left everything alone but the case
that was segfaulting my fuzz test. :-)

----------------------------------------------------------------------

Comment By: Michiel de Hoon (mdehoon)
Date: 2005-01-21 03:20

Message:
Logged In: YES 
user_id=488897

I have run the test suite after applying this patch, and I
found no problems with it. The bug does indeed originate in
the readline library, which does not return NULL if the
index is negative. I sent a patch to bug-readline at gnu.org,
so this will probably be fixed in future versions of
readline. But I agree that for now, we need a workaround in
Python.
Note that there is one more way to fix this bug, which is to
interpret negative indeces as counting from the end. So
remove_history_item(-1) removes the last item added to the
history etc. This would be more consistent with lists etc.
in Python, and users may even expect this behavior.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1093585&group_id=5470