From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9224 invoked by alias); 26 May 2017 09:30:14 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 41164 Received: (qmail 12949 invoked from network); 26 May 2017 09:30:13 -0000 X-Qmail-Scanner-Diagnostics: from mailout4.w1.samsung.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(210.118.77.14):SA:0(-5.0/5.0):. Processed in 2.68673 secs); 26 May 2017 09:30:13 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-5.0 required=5.0 tests=RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: p.stephenson@samsung.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at samsung.com does not designate permitted sender hosts) X-AuditID: cbfec7f2-f797e6d000004438-4e-5927f3415d63 Date: Fri, 26 May 2017 10:19:57 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: zsh 5.3.1 crashes on completion Message-id: <20170526101957.126606d2@pwslap01u.europe.root.pri> In-reply-to: Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrDIsWRmVeSWpSXmKPExsWy7djP87qOn9UjDdY1i1gcbH7I5MDoserg B6YAxigum5TUnMyy1CJ9uwSujOVXOpkL3hlVTN/9gaWB8Y9aFyMnh4SAicTtiT9ZIWwxiQv3 1rOB2EICSxklNn0I72LkArJ7mSSunbzFDNMw/9AONojEMkaJlj3v2SGcaUwSU489gcqcYZS4 2zoDyjnLKDHx1iuwwSwCqhIHDyxiArHZBAwlpm6azQhiiwiIS5xde56li5GDQ1hAR2L//gKQ MK+AvcSdSV/BVnMKuEpcn3GWHcTmF9CXuPr3ExPESfYSM6+cYYSoF5T4MfkeC4jNDDRm27bH 7BC2vMTmNW+ZQe6REGhml1gzGWKXhICsxKYDUK+5SBw62gYNC2GJV8e3sEPYMhKXJ3ezQNj9 jBJPun0h5sxglDh9ZgcbRMJaou/2RUaIZXwSk7ZNZ4aYzyvR0SYEUeIhsfhlEyOE7Sixf8pM 1gmMirOQnD0LydmzkJy9gJF5FaNIamlxbnpqsbFecWJucWleul5yfu4mRmAaOP3v+KcdjF9P WB1iFOBgVOLh3XBPLVKINbGsuDL3EKMEB7OSCO+al+qRQrwpiZVVqUX58UWlOanFhxilOViU xHm5Tl2LEBJITyxJzU5NLUgtgskycXBKNTBu54sQ8c9fG5UrybFh75zdpwP+TW5czTZnpoa5 3GOrKcmq1qcYjJU4Dzl+TJKZoLrAyFXS6Yj25aidkoGBF7vntAQeFz/Pe71YttnDzH/GEYd2 XuWDYd8mize4rdR4MvNra17T8iQlVSEt5SPZ7wqci/czalv0tUxcufr0PHdFkewJ++TN5iix FGckGmoxFxUnAgDvl8jH/wIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrKIsWRmVeSWpSXmKPExsVy+t/xK7oOn9UjDd53y1ocbH7I5MDoserg B6YAxig3m4zUxJTUIoXUvOT8lMy8dFul0BA3XQslhbzE3FRbpQhd35AgJYWyxJxSIM/IAA04 OAe4Byvp2yW4ZSy/0slc8M6oYvruDywNjH/Uuhg5OSQETCTmH9rBBmGLSVy4tx7I5uIQEljC KNG45CY7hDODSWLdjidMEM45RolN9yewQjhnGSUWzbrLCNLPIqAqcfDAIiYQm03AUGLqptlg cREBcYmza8+zdDFycAgL6Ejs318AEuYVsJe4M+krM4jNKeAqcX3GWXYQW0jgKYvE9qW+IDa/ gL7E1b+fmCDOs5eYeeUMI0SvoMSPyfdYQGxmAS2JzduaWCFseYnNa94yQ8xRl7hxdzf7BEbh WUhaZiFpmYWkZQEj8ypGkdTS4tz03GIjveLE3OLSvHS95PzcTYzAKNp27OeWHYxd74IPMQpw MCrx8HI8VIsUYk0sK67MPcQowcGsJMK75qV6pBBvSmJlVWpRfnxRaU5q8SFGU2C4TGSWEk3O B0Z4Xkm8oYmhuaWhkbGFhbmRkZI479QPV8KFBNITS1KzU1MLUotg+pg4OKUaGFM/RVnv5Pol Z7cpfk5LzadYsUYzq+aUc80+tgf+HWIz0jysyXrmz1npZ8wBbs9b9n2wupB+b9Wri8s+v+CY WpJXPjU/+TezUP7/c0/YGJ1rpy+N6ZlesirsQ6SSa3in1PqfnS9/ub3+0OXjq3k1RkTyYxXn pEXfX+asP8K9w6Jrbvzn+qwN05RYijMSDbWYi4oTAd/0+qu4AgAA X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170526092000eucas1p24922da74ce8fdf679e2cceaaf26a5459 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUb?= =?UTF-8?B?7IK87ISx7KCE7J6QG1ByaW5jaXBhbCBFbmdpbmVlciwgU29mdHdhcmU=?= X-Global-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUbU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtQcmluY2lwYWwgRW5naW5lZXIsIFNvZnR3YXJl?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDA1Q0QwNTAwNTg=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170513182335epcas3p2ddebe4bda1f5c273cce7a5902d621aa7 X-RootMTR: 20170513182335epcas3p2ddebe4bda1f5c273cce7a5902d621aa7 References: <170511141903.ZM13639@torch.brasslantern.com> <20170512161603.GA16688@fujitsu.shahaf.local2> <783A691B-B69F-4673-B237-B316CEF26346@remrain.com> <170513112313.ZM14712@torch.brasslantern.com> <20170515102819.39284ef6@pwslap01u.europe.root.pri> <170515133354.ZM20191@torch.brasslantern.com> <20170516104831.14aa23c6@pwslap01u.europe.root.pri> <20170516120527.572a5cc9@pwslap01u.europe.root.pri> On Fri, 26 May 2017 01:01:16 +0900 Jun T. wrote: > With the latest git master, expand-history doesn't work correctly: > > [1]% echo one > [2]% echo two > [3]% !! (or the key bound to expand-history) > > then !! expands to 'echo one' (!! works as expected). There are two obvious ways to go here. First, link and unlink the current history line into the ring at additional points when doing interactive expansion or completion. Although this might well be safer in terms of structure it's quite hard to track down the right points. I haven't looked further. Second, instead of using link / unlink, directly mark the current history in some way so that it doesn't get freed. As curline's not on the saved history stack, the assumption (whether safe or not is an entirely different matter) is that it's unique, i.e. there are not going to be different notions of it at different lexical level. In that case simply comparing against it so we don't free it will fix the immediate problem. This doesn't keep the structure at different nesting levels as separate, but it looks like we can't. Here's a possible minimal fix to the original problem based on that --- that means I really don't know if there are other ramifications that may turn up, but it looks like we need to track them down one by one rather than assuming structure the shell doesn't have. The first change is the actual fix, making the DPUTS as well as the other new framework redundant. pws diff --git a/Src/hashtable.c b/Src/hashtable.c index c34744c..6ec2ed2 100644 --- a/Src/hashtable.c +++ b/Src/hashtable.c @@ -1445,10 +1445,12 @@ freehistdata(Histent he, int unlink) if (!he) return; + if (he == &curline) + return; + if (!(he->node.flags & (HIST_DUP | HIST_TMPSTORE))) removehashnode(histtab, he->node.nam); - DPUTS(he->node.nam == chline, "Attempt to free chline in history data"); zsfree(he->node.nam); if (he->nwords) zfree(he->words, he->nwords*2*sizeof(short)); diff --git a/Src/hist.c b/Src/hist.c index 4c1039b..350688d 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -87,9 +87,6 @@ mod_export zlong curhist; /**/ struct histent curline; -/***/ -int curline_linked; - /* current line count of allocated history entries */ /**/ @@ -264,9 +261,6 @@ hist_context_save(struct hist_stack *hs, int toplevel) */ hs->cstack = cmdstack; hs->csp = cmdsp; - hs->curline_linked = curline_linked; - - unlinkcurline(); stophist = 0; chline = NULL; @@ -306,9 +300,6 @@ hist_context_restore(const struct hist_stack *hs, int toplevel) zfree(cmdstack, CMDSTACKSZ); cmdstack = hs->cstack; cmdsp = hs->csp; - unlinkcurline(); - if (hs->curline_linked) - linkcurline(); } /* @@ -999,7 +990,6 @@ nohwe(void) /* these functions handle adding/removing curline to/from the hist_ring */ -/**/ static void linkcurline(void) { @@ -1012,15 +1002,11 @@ linkcurline(void) hist_ring = &curline; } curline.histnum = ++curhist; - curline_linked = 1; } -/**/ static void unlinkcurline(void) { - if (!curline_linked) - return; curline.up->down = curline.down; curline.down->up = curline.up; if (hist_ring == &curline) { @@ -1030,7 +1016,6 @@ unlinkcurline(void) hist_ring = curline.up; } curhist--; - curline_linked = 0; } /* initialize the history mechanism */ @@ -1050,7 +1035,6 @@ hbegin(int dohist) stophist = (!interact || unset(SHINSTDIN)) ? 2 : 0; else stophist = 0; - DPUTS(chline != NULL, "chline set at start of history"); /* * pws: We used to test for "|| (inbufflags & INP_ALIAS)" * in this test, but at this point we don't have input @@ -1308,10 +1292,11 @@ putoldhistentryontop(short keep_going) Histent prepnexthistent(void) { - Histent he; - int relink_curline = curline_linked; + Histent he; + int curline_in_ring = hist_ring == &curline; - unlinkcurline(); + if (curline_in_ring) + unlinkcurline(); if (hist_ring && hist_ring->node.flags & HIST_TMPSTORE) { curhist--; freehistnode(&hist_ring->node); @@ -1335,7 +1320,7 @@ prepnexthistent(void) he = hist_ring; } he->histnum = ++curhist; - if (relink_curline) + if (curline_in_ring) linkcurline(); return he; } @@ -1410,7 +1395,8 @@ hend(Eprog prog) queue_signals(); if (histdone & HISTFLAG_SETTY) settyinfo(&shttyinfo); - unlinkcurline(); + if (!(histactive & HA_NOINC)) + unlinkcurline(); if (histactive & HA_NOINC) { zfree(chline, hlinesz); zfree(chwords, chwordlen*sizeof(short)); @@ -3638,7 +3624,7 @@ int pushhiststack(char *hf, zlong hs, zlong shs, int level) { struct histsave *h; - int relink_curline = curline_linked; + int curline_in_ring = (histactive & HA_ACTIVE) && hist_ring == &curline; if (histsave_stack_pos == histsave_stack_size) { histsave_stack_size += 5; @@ -3646,7 +3632,8 @@ pushhiststack(char *hf, zlong hs, zlong shs, int level) histsave_stack_size * sizeof (struct histsave)); } - unlinkcurline(); + if (curline_in_ring) + unlinkcurline(); h = &histsave_stack[histsave_stack_pos++]; @@ -3681,7 +3668,7 @@ pushhiststack(char *hf, zlong hs, zlong shs, int level) savehistsiz = shs; inithist(); /* sets histtab */ - if (relink_curline) + if (curline_in_ring) linkcurline(); return histsave_stack_pos; @@ -3693,12 +3680,13 @@ int pophiststack(void) { struct histsave *h; - int relink_curline = curline_linked; + int curline_in_ring = (histactive & HA_ACTIVE) && hist_ring == &curline; if (histsave_stack_pos == 0) return 0; - unlinkcurline(); + if (curline_in_ring) + unlinkcurline(); deletehashtable(histtab); zsfree(lasthist.text); @@ -3721,7 +3709,7 @@ pophiststack(void) histsiz = h->histsiz; savehistsiz = h->savehistsiz; - if (relink_curline) + if (curline_in_ring) linkcurline(); return histsave_stack_pos + 1; diff --git a/Src/zsh.h b/Src/zsh.h index 405b274..22f73f8 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -2931,7 +2931,6 @@ struct hist_stack { void (*addtoline) _((int)); unsigned char *cstack; int csp; - int curline_linked; }; /*