Re: Calling Lisp from undo.c's record_* functions

[Eli Zaretskii]

> From: Stefan Monnier <address@hidden>
> Cc: address@hidden,  address@hidden
> Date: Tue, 17 Nov 2015 12:34:08 -0500
> 
> >> When would it not be called?
> > E.g., in insert_from_gap.  Also, in many insdel.c functions when they
> > are called with the PREPARE argument false.
> 
> AFAIK these are cases where prepare_to_modify_buffer has already been
> called earlier.

That's what the comments say, yes.  How deep do we believe them?

> >> You mean there are cases where we'd add stuff to the undo list but
> >> we don't run before-change-functions?
> > I don't know.  I don't think we have such bugs, but thinking is one
> > thing and convincing yourself it's true by looking at the callers is
> > something entirely different...
> 
> That's OK, then: I believe that failing to call run_undoable_change is
> not more serious than failing to run before-change-functions.
> 
> So I think moving the call to run_undoable_change into
> prepare_to_modify_buffer is n attractive solution to this problem, since
> it preserves the use of Elisp, and it probably also simplifies the code
> (since we can remove most/all other calls to run_undoable_change).

Did you take a look at subst-char-in-region?  It calls
prepare_to_modify_buffer from within a loop which seems to assume that
(a) gap position doesn't move, and (b) that pointer into buffer text
is valid across the call to prepare_to_modify_buffer.  GC could
invalidate both assumptions, no?

transpose-regions also does something funky, but it looks safe.

zlib-decompress-region, OTOH, seems to call insert_from_gap, and
doesn't call prepare_to_modify_buffer at all, except at its very end,
when it deletes the uncompressed data.

Bottom line: I think all the functions that manipulate the gap should
be carefully audited to identify any potential problem with this
approach.