From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2689 invoked by alias); 9 Dec 2016 17:33:46 -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: 40141 Received: (qmail 11072 invoked from network); 9 Dec 2016 17:33:46 -0000 X-Qmail-Scanner-Diagnostics: from mx2.imag.fr 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(129.88.30.17):SA:0(-5.3/5.0):. Processed in 1.494272 secs); 09 Dec 2016 17:33:46 -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.3 required=5.0 tests=RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: raphael.jakse@imag.fr X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: pass (ns1.primenet.com.au: SPF record at imag.fr designates 129.88.30.17 as permitted sender) Subject: Re: [PATCH] Fix #87 - Segfault fault when autocompleting after ">" in, "!> ." To: Peter Stephenson , Bart Schaefer References: <53d2b131-bca8-b968-bf46-fa4e4d801231@imag.fr> <20161128165737.09522ec2@pwslap01u.europe.root.pri> Cc: zsh-workers@zsh.org From: =?UTF-8?Q?Rapha=c3=abl_Jakse?= Message-ID: Date: Fri, 9 Dec 2016 17:42:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20161128165737.09522ec2@pwslap01u.europe.root.pri> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.2 (mx2.imag.fr [129.88.30.17]); Fri, 09 Dec 2016 17:42:58 +0100 (CET) X-IMAG-MailScanner-Information: Please contact MI2S MIM for more information X-MailScanner-ID: uB9Ggv0J027027 X-IMAG-MailScanner: Found to be clean X-IMAG-MailScanner-SpamCheck: X-IMAG-MailScanner-From: raphael.jakse@imag.fr MailScanner-NULL-Check: 1481906578.66744@2ZiHaM5t8M78b1x5uUI77g Bart, Peter, Thanks for taking a look at it. Both of your patches seem more accurate than mine. It would be nice if one of them could make it into zsh. For your information, looking at this bug was / is part of my research. We try to combine monitoring techniques and interactive debugging. If you are interested, feel free to ask me questions and look at our our tool here: http://check-exec.forge.imag.fr/ Best, Raphaël Le 28/11/2016 à 17:57, Peter Stephenson a écrit : > On Mon, 28 Nov 2016 14:06:54 +0100 > Raphaël Jakse wrote: >> Bug #87 in the SF bug tracker (which is not used anymore if I understood >> correctly) claims that when hitting tab to complete after '>' in the >> string "!> cmake ..", zsh crashes. I could reproduce the bug that also >> appears with the string "!> .". > Thanks, that's still the case and this is indeed a tricky piece of code. > Sending patches here is the right thing to do. > > I think the following patch does a little better: I've tracked down the > case where this is failing, and it's in handling something that looks > like a redirection before the command word. Here it looks a bit like a > redirection because of the > but not enough because of the !. > > I haven't fully resolved redirection handling here (not sure exactly > what this mixture should really do at this point), but I have made the > particular aspect of it that was causing the problem a bit safer: > because the code here didn't properly know about the redirection it > thought it could reset the completion word when it found the command > word. But because the lexer had detected a redirection and we'd > already passed the word where the cursor was, that was no longer safe. > Detecting we've already found the completion word looks like an > excellent reason for leaving the command line word array that we've > built up so far alone whatever else is happening. > > I've also added a debug message if we find a string that's NULL, which > should also make it safe in the way you intended but with debug info for > developers if debugging is turned on. > > The middle hunk is a bit of extra safety / readability. Possibly some > extra DPUTS() would be a good idea there. > > With a bit of luck (we don't tend to get much in get_comp_string()), the > condition is specific enough that this doesn't make anything else worse. > > pws > > diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c > index c8d3bb3..3d86791 100644 > --- a/Src/Zle/zle_tricky.c > +++ b/Src/Zle/zle_tricky.c > @@ -1305,8 +1305,12 @@ get_comp_string(void) > zsfree(cmdstr); > cmdstr = ztrdup(tokstr); > cmdtok = tok; > - /* If everything before is a redirection, don't reset the index */ > - if (wordpos != redirpos) > + /* > + * If everything before is a redirection, or anything > + * complicated enough that we've seen the word the > + * cursor is on, don't reset the index. > + */ > + if (wordpos != redirpos && clwpos == -1) > wordpos = redirpos = 0; > } else if (tok == SEPER) { > /* > @@ -1414,9 +1418,17 @@ get_comp_string(void) > /* If this is the word the cursor is in and we added a `x', * > * remove it. */ > if (clwpos == wordpos++ && addedx) { > + int chuck_at, word_diff; > zlemetacs_qsub = zlemetacs - qsub; > - chuck(&clwords[wordpos - 1][((zlemetacs_qsub - wb) >= sl) ? > - (sl - 1) : (zlemetacs_qsub - wb)]); > + word_diff = zlemetacs_qsub - wb; > + /* Ensure we chuck within the word... */ > + if (word_diff >= sl) > + chuck_at = sl -1; > + else if (word_diff < 0) > + chuck_at = 0; > + else > + chuck_at = word_diff; > + chuck(&clwords[wordpos - 1][chuck_at]); > } > } while (tok != LEXERR && tok != ENDINPUT && > (tok != SEPER || (lexflags && tt0 == NULLTOK))); > @@ -1464,7 +1476,9 @@ get_comp_string(void) > t0 = STRING; > } else if (t0 == STRING || t0 == TYPESET) { > /* We found a simple string. */ > - s = ztrdup(clwords[clwpos]); > + s = clwords[clwpos]; > + DPUTS(!s, "Completion word has disappeared!"); > + s = ztrdup(s ? s : ""); > } else if (t0 == ENVSTRING) { > char sav; > /* The cursor was inside a parameter assignment. */