From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template =?UTF-8?Q?=E2=80=98g=E2=80=99:?= Match data clobbered by buffer modification hooks) Date: Tue, 19 Jul 2016 00:48:19 -0400 Message-ID: References: <87vb066ejv.fsf@linaro.org> <8360s67qcp.fsf@gnu.org> <87bn1yyaui.fsf@linaro.org> <87mvlhmv0x.fsf_-_@moondust.awandering> <837fcl5zs9.fsf@gnu.org> <87a8hgkwcb.fsf@linaro.org> <8360s42mcb.fsf@gnu.org> <87eg6rgmlg.fsf@gmail.com> <83lh0y24y6.fsf@gnu.org> <83eg6q1hbo.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1468903771 16825 80.91.229.3 (19 Jul 2016 04:49:31 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 19 Jul 2016 04:49:31 +0000 (UTC) Cc: 23917@debbugs.gnu.org, rpluim@gmail.com, jwiegley@gmail.com, alex.bennee@linaro.org, nljlistbox2@gmail.com To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Jul 19 06:49:19 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bPMy6-0001AR-TZ for geb-bug-gnu-emacs@m.gmane.org; Tue, 19 Jul 2016 06:49:19 +0200 Original-Received: from localhost ([::1]:51981 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPMy6-0005He-1p for geb-bug-gnu-emacs@m.gmane.org; Tue, 19 Jul 2016 00:49:18 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPMxy-0005Cv-4s for bug-gnu-emacs@gnu.org; Tue, 19 Jul 2016 00:49:12 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPMxq-0002gF-MJ for bug-gnu-emacs@gnu.org; Tue, 19 Jul 2016 00:49:10 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:43954) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPMxq-0002g1-IZ; Tue, 19 Jul 2016 00:49:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bPMxq-0006eF-DJ; Tue, 19 Jul 2016 00:49:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org, emacs-orgmode@gnu.org Resent-Date: Tue, 19 Jul 2016 04:49:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23917 X-GNU-PR-Package: emacs,org-mode X-GNU-PR-Keywords: Original-Received: via spool by 23917-submit@debbugs.gnu.org id=B23917.146890370925515 (code B ref 23917); Tue, 19 Jul 2016 04:49:02 +0000 Original-Received: (at 23917) by debbugs.gnu.org; 19 Jul 2016 04:48:29 +0000 Original-Received: from localhost ([127.0.0.1]:56291 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bPMxJ-0006dS-9d for submit@debbugs.gnu.org; Tue, 19 Jul 2016 00:48:29 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:7021) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bPMxH-0006dE-1R for 23917@debbugs.gnu.org; Tue, 19 Jul 2016 00:48:27 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0AmDgA731xV/3mcpUVcgxCEAghGxUWCTQQCAoE8PRABAQEBAQEBgQpBBYNdAQEDAQwXMyMFCwkCGgIYBwcCAhQYDRMRiDcIjXydEKQXAQEBBwEBAQEegSGKGYRSMweCaIFFAQSfF4NrkD2BRSOBZlWBWSKCeAEBAQ X-IPAS-Result: A0AmDgA731xV/3mcpUVcgxCEAghGxUWCTQQCAoE8PRABAQEBAQEBgQpBBYNdAQEDAQwXMyMFCwkCGgIYBwcCAhQYDRMRiDcIjXydEKQXAQEBBwEBAQEegSGKGYRSMweCaIFFAQSfF4NrkD2BRSOBZlWBWSKCeAEBAQ X-IronPort-AV: E=Sophos;i="5.13,465,1427774400"; d="scan'208";a="248473132" Original-Received: from 69-165-156-121.dsl.teksavvy.com (HELO fmsmemgm.homelinux.net) ([69.165.156.121]) by ironport2-out.teksavvy.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 19 Jul 2016 00:48:19 -0400 Original-Received: by fmsmemgm.homelinux.net (Postfix, from userid 20848) id AF1F9AE2BB; Tue, 19 Jul 2016 00:48:19 -0400 (EDT) In-Reply-To: <83eg6q1hbo.fsf@gnu.org> (Eli Zaretskii's message of "Tue, 19 Jul 2016 05:40:11 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:121254 Archived-At: > The more general problem is when there's at least one more > sub-expression, whose start and/or end are after the new EOB. > Those sub-expression's data will be completely bogus after the > adjustment, If they were after the EOB, they were already bogus to start with. So there's really not much harm moving them around. And in any case, that's what has been happening for ever and has proved safe enough. >> So I think a safe fix is to try and relax the check we added to >> replace-match so it doesn't get all worked up when something =E2=89=A5 E= OB gets >> changed to something else that's also =E2=89=A5 EOB. > And lose the other sub-expressions in a more general case? Really? I'm not sure what you mean by "losing sub-expressions". But yes, I think the behavior of save-match-data you describe is not a real problem. Arguably if save-match-data moves positions from outside BEGV...ZV to inside it it's a problem. But if it moves them from outside BEG...Z to inside it, I think it's perfectly fine. >> Or maybe instead of signaling an error, we could simply skip the "Adjust >> search data for this change". > That would still sweep the problem under the carpet, leaving the match > data bogus, so I don't like doing that. Maybe I'm not 100% satisfied with the behavior either, but I don't think it's a significant problem and I don't think it'd cause the crash we saw in bug#23869. > The crash in bug#23869 was due to this: > > newpoint =3D search_regs.start[sub] + SCHARS (newtext); > [...] > /* Now move point "officially" to the start of the inserted replacement= . */ > move_if_not_intangible (newpoint); <<<<<<<<<<<<<<<<<<<<<<< > > because due to clobbering, newpoint became -1. Ah, I see. Then maybe another fix is to compute newpoint before we call replace_range, so it uses search_regs.start[sub] before the *-change-functions can mess it up. IOW: @@ -2726,9 +2726,9 @@ since only regular expressions have distinguished= subexpressions. */) unsigned num_regs =3D search_regs.num_regs; =20=20=20=20=20 /* Replace the old text with the new in the cleanest possible way. = */ + newpoint =3D search_regs.start[sub] + SCHARS (newtext); replace_range (search_regs.start[sub], search_regs.end[sub], newtext, 1, 0, 1); - newpoint =3D search_regs.start[sub] + SCHARS (newtext); =20=20=20=20=20 if (case_action =3D=3D all_caps) Fupcase_region (make_number (search_regs.start[sub]), Would that be sufficient to avoid the crash? Why not? Stefan