emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 23917@debbugs.gnu.org, rpluim@gmail.com, jwiegley@gmail.com,
	alex.bennee@linaro.org, nljlistbox2@gmail.com
Subject: bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks)
Date: Tue, 19 Jul 2016 18:36:08 +0300	[thread overview]
Message-ID: <838twx1vyv.fsf@gnu.org> (raw)
In-Reply-To: <jwvmvle5udz.fsf-monnier+emacsbugs@gnu.org> (message from Stefan Monnier on Mon, 18 Jul 2016 20:58:35 -0400)

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Robert Pluim <rpluim@gmail.com>,  23917@debbugs.gnu.org,  alex.bennee@linaro.org,  jwiegley@gmail.com,  nljlistbox2@gmail.com
> Date: Mon, 18 Jul 2016 20:58:35 -0400
> 
> > I think this change performed by save-match-data is harmless: the old
> > value (62) was not valid any more anyway.
> 
> In this particular case, yes.  But only in this case, because (a)
> there's actually only one sub-expression, and (b) it ends exactly at
> EOB.

Moreover, if the value of 62 was left alone by save-match-data, the
adjustment code in replace-match would have fixed it.  That's what
that code is all about: it fixes the match data due to changes to the
buffer made by replacing the old text by the new.  Any attempts to
"fix" those values under that code's feet are not helpful; quite the
contrary.

> > 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 ≥ EOB gets
> > changed to something else that's also ≥ EOB.
> 
> And lose the other sub-expressions in a more general case?  Really?

What really bothers me is that by just loosening the conditions under
which we signal an error we defeat _valid_ code, which did use
save-match-data, and yet the match data still ends up being
clobbered.  I don't mind making the test more loose so that invalid
programs have a longer rope to hang themselves, but valid programs
should not be failed, IMO.

> > 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.

> > This said, I don't fully understand what's going on: bug#23869 reported
> > a crash, but AFAICT the match-data here is only used to adjust
> > search_regs which seems like it wouldn't cause a crash, even if the new
> > values are bogus.

> The crash in bug#23869 was due to this:

>   newpoint = 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.


> > > -   '((save-match-data-internal (match-data)))
> > > +   '((save-match-data-internal (match-data 'integers)))
> > 
> > That looks risky.
> 
> Then how about manually doing the equivalent of save-match-data around
> the call to replace_range, calling match-data with non-nil argument?

Here are some more alternatives for dealing with this issue:

 (1) Make match-data use integers instead of markers by default when a
     call to replace-match is in progress, i.e. when match-data is
     called from some buffer-modification hook triggered by
     replace-match

 (2) Don't signal an error, even if match data seems clobbered, if the
     new value of point is valid and either (a) there's only one
     search register, or (b) the adjustment value is zero (i.e. the
     registers will be left unchanged).

I like (1) better than (2) because (1) will let valid programs avoid
clobbering match data, but maybe it's too risky for emacs-25.  Also,
is it plausible that some buffer-modification hook will edit the
buffer in the save-match-data forms, and expect the match data to be
adjusted, or is this something that a buffer-modification hook should
never do?  If valid programs can do this, then (1) is probably not a
good idea.

  parent reply	other threads:[~2016-07-19 15:37 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <87vb066ejv.fsf@linaro.org>
     [not found] ` <8360s67qcp.fsf@gnu.org>
     [not found]   ` <87bn1yyaui.fsf@linaro.org>
     [not found]     ` <CAM-tV-9Befm+jtvvO9tZsYwCbzgggumNgWeuPzV-i=ZB0mgPog@mail.gmail.com>
     [not found]       ` <87mvlhmv0x.fsf_-_@moondust.awandering>
     [not found]         ` <837fcl5zs9.fsf@gnu.org>
     [not found]           ` <m28tx1o6h6.fsf@newartisans.com>
     [not found]             ` <87a8hgkwcb.fsf@linaro.org>
     [not found]               ` <8360s42mcb.fsf@gnu.org>
2016-07-18 12:24                 ` bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks) Robert Pluim
2016-07-18 14:50                 ` Kaushal Modi
     [not found]                 ` <9613abde4dbd48fdb2b5e780101a13a0@HE1PR01MB1898.eurprd01.prod.exchangelabs.com>
2016-07-18 16:59                   ` Eric S Fraga
     [not found]                 ` <87eg6rgmlg.fsf@gmail.com>
2016-07-18 18:09                   ` Eli Zaretskii
     [not found]                   ` <83lh0y24y6.fsf@gnu.org>
2016-07-18 19:04                     ` John Wiegley
2016-07-18 19:10                       ` Eli Zaretskii
2016-07-19  0:58                     ` Stefan Monnier
     [not found]                     ` <jwvmvle5udz.fsf-monnier+emacsbugs@gnu.org>
2016-07-19  2:40                       ` Eli Zaretskii
     [not found]                       ` <83eg6q1hbo.fsf@gnu.org>
2016-07-19  4:48                         ` Stefan Monnier
     [not found]                         ` <jwv1t2q5jsz.fsf-monnier+emacsbugs@gnu.org>
2016-07-19 15:35                           ` Eli Zaretskii
     [not found]                           ` <83a8hd1vzi.fsf@gnu.org>
2016-07-19 16:03                             ` Stefan Monnier
     [not found]                             ` <jwvzipdeibv.fsf-monnier+emacsbugs@gnu.org>
2016-07-19 16:13                               ` Eli Zaretskii
     [not found]                               ` <834m7l1u8u.fsf@gnu.org>
2016-07-19 17:05                                 ` bug#23917: " Alex Bennée
     [not found]                                 ` <87a8hdmuce.fsf@linaro.org>
2016-07-19 17:20                                   ` Eli Zaretskii
     [not found]                                   ` <8337n51r4f.fsf@gnu.org>
2016-07-19 17:45                                     ` Alex Bennée
     [not found]                                     ` <878twxmshj.fsf@linaro.org>
2016-07-19 18:07                                       ` Sebastian Wiesner
2016-07-19 18:44                                       ` Eli Zaretskii
2016-07-20  9:48                                         ` bug#23917: " Alex Bennée
     [not found]                                         ` <877fcgmygy.fsf@linaro.org>
2016-07-20 14:59                                           ` Eli Zaretskii
2016-07-20  1:50                                 ` Stefan Monnier
     [not found]                                 ` <jwvd1m9dran.fsf-monnier+emacsbugs@gnu.org>
2016-07-20 14:55                                   ` Eli Zaretskii
     [not found]                                   ` <83shv4z7e8.fsf@gnu.org>
2016-07-20 18:19                                     ` Stefan Monnier
2016-07-20 18:55                                       ` Eli Zaretskii
     [not found]                                       ` <83inw0yw9q.fsf@gnu.org>
2016-07-20 20:54                                         ` Stefan Monnier
     [not found]                                         ` <jwvtwfkvxsc.fsf-monnier+emacsbugs@gnu.org>
2016-07-21  0:56                                           ` npostavs
     [not found]                                           ` <87bn1rdd1f.fsf@users.sourceforge.net>
2016-07-21  1:47                                             ` Stefan Monnier
     [not found]                                             ` <jwvzipb3gx0.fsf-monnier+emacsbugs@gnu.org>
2016-07-21  2:34                                               ` Noam Postavsky
     [not found]                                               ` <CAM-tV-9rqqOF=zjsXx3QQ5_z0n-fFB7-BCbxWdzb9EA_u2ZHPQ@mail.gmail.com>
2016-07-21  3:06                                                 ` Stefan Monnier
2016-07-21  2:43                                             ` Eli Zaretskii
     [not found]                                             ` <83h9bjzp5i.fsf@gnu.org>
2016-07-21  3:00                                               ` npostavs
     [not found]                                               ` <877fcfd79w.fsf@users.sourceforge.net>
2016-07-21 14:26                                                 ` Eli Zaretskii
     [not found]                                                 ` <83a8hbysmw.fsf@gnu.org>
2016-07-22  1:08                                                   ` npostavs
     [not found]                                                   ` <874m7icwdg.fsf@users.sourceforge.net>
2016-07-22  6:43                                                     ` Eli Zaretskii
     [not found]                                                     ` <83fur2xjeu.fsf@gnu.org>
2016-07-22 14:01                                                       ` Robert Pluim
2016-07-22 19:30                                                       ` Nicolas Petton
2016-07-23  4:19                                                       ` npostavs
2016-07-23  0:42                                                     ` bug#23917: Re: bug#23917: 25.0.95; commit 3a9d6296b35e5317c497674d5725eb52699bd3b8 causing org-capture to error out N. Jackson
     [not found]                                                     ` <878twtqj6n.fsf_-_@moondust.awandering>
2016-07-23  7:38                                                       ` Eli Zaretskii
2016-07-21  7:59                                             ` N. Jackson
     [not found]                                             ` <87a8hb2zgt.fsf_-___1286.15575745261$1469088239$gmane$org@moondust.awandering>
2016-07-21  8:19                                               ` Robert Pluim
     [not found]                                                 ` <8760sgnvxh.fsf@gmail.com>
     [not found]                                                   ` <83lh1ci5x5.fsf@gnu.org>
     [not found]                                                     ` <87poqo6sud.fsf@gmail.com>
2016-07-08 17:03                                                       ` Eli Zaretskii
     [not found]                                                       ` <83a8hshxjs.fsf@gnu.org>
2016-07-15 19:46                                                         ` Eli Zaretskii
2016-07-21 14:24                                                   ` N. Jackson
2016-07-19 23:18                             ` bug#23917: Please consider making Bug #23917 a blocker for 25.1 (was Re: org-capture: Capture template ‘g’: Match data clobbered by buffer modification hooks) npostavs
2016-07-19 15:36                       ` Eli Zaretskii [this message]
2016-07-21  7:52                     ` bug#23917: 25.0.95; commit 3a9d6296b35e5317c497674d5725eb52699bd3b8 causing org-capture to error out N. Jackson
     [not found]                     ` <87h9bj2zsl.fsf_-_@moondust.awandering>
2016-07-21  8:08                       ` Robert Pluim
     [not found]                       ` <87poq7xvjy.fsf@gmail.com>
2016-07-21 13:19                         ` N. Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=838twx1vyv.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=23917@debbugs.gnu.org \
    --cc=alex.bennee@linaro.org \
    --cc=jwiegley@gmail.com \
    --cc=monnier@iro.umontreal.ca \
    --cc=nljlistbox2@gmail.com \
    --cc=rpluim@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).