Re: Lisp primitives and their calling of the change hooks

[Alan Mackenzie]

Hello, Eli.

On Mon, Jan 08, 2018 at 05:41:02 +0200, Eli Zaretskii wrote:
> > Date: Sun, 7 Jan 2018 21:10:55 +0000
> > Cc: address@hidden, address@hidden
> > From: Alan Mackenzie <address@hidden>

> > > Like I said, I think we should have only one pair of hooks, like we
> > > have in replace_range.

> > OK.  This is slightly more awkward: del_range needs to be replaced by
> > del_range_2, so as not to get an extra call to signal_after_change.

> You need a call to update_compositions after del_range_2, to mimic
> what del_range does, I think.

Yes, I think so to (though I'm only vaguely aware of what compositions
are ;-).  See below.

> > There is also no a-c-f call if the decompression exits with an error.

> You mean, if the user quits?  That throws to top level, so it would be
> wrong to invoke any after-change hooks, and unwind_decompress will
> call the hooks for the partially uncompressed data.  Do we need more?

I was thinking more of when the compressed text is corrupt and the
decompression routines report an error.  With the current version (see
below), I simulate an error with

    (zlib-decompress-region (point-min) (1- (point-max)))

, i.e. chopping the last byte off the buffer.  (I am using my Linux
configuration from /proc/config.gz for all this.)

This produces these hook calls:

    (1 22016)    (22016 118057) (22016 22016 96041)

, the first being the opening modify_text(), and the last two being the
deletion of the incomplete decompressed region by a call to del_range()
from an unwind-protect.  The (1 22016) b-c-f is thus unbalanced when this
happens.

I'm asking you to consider again having two pairs of hook calls in this
primitive (as, for example, base64-decode-region does).  That way we need
only signal the b-c-f for the deletion after the decompression has
worked, and we know we are going to follow through with the deleteion.  I
think an aborted decompression operation would also be easier to close
off with an a-c-f with this strategy.

Here's the amended patch from last night with update_compositions in:



diff --git a/src/decompress.c b/src/decompress.c
index 41de6da1dd..eebaa2eb30 100644
--- a/src/decompress.c
+++ b/src/decompress.c
@@ -24,6 +24,7 @@ along with GNU Emacs.  If not, see 
<https://www.gnu.org/licenses/>.  */
 
 #include "lisp.h"
 #include "buffer.h"
+#include "composite.h"
 
 #include <verify.h>
 
@@ -141,6 +142,10 @@ This function can be called only in unibyte buffers.  */)
      the same.  */
   istart = XINT (start);
   iend = XINT (end);
+
+  /* Do the following before manipulating the gap. */
+  modify_text (istart, iend);
+
   move_gap_both (iend, iend);
 
   stream.zalloc = Z_NULL;
@@ -196,7 +201,11 @@ This function can be called only in unibyte buffers.  */)
   unwind_data.start = 0;
 
   /* Delete the compressed data.  */
-  del_range (istart, iend);
+  del_range_2 (istart, istart, /* byte and char offsets are the same. */
+               iend, iend, 0);
+
+  signal_after_change (istart, iend - istart, unwind_data.nbytes);
+  update_compositions (istart, istart, CHECK_HEAD);
 
   return unbind_to (count, Qt);
 }


> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).