GNU bug report logs - #36431
Crash in marker.c:337

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: wl <at> gnu.org, 36431 <at> debbugs.gnu.org
Subject: bug#36431: Crash in marker.c:337
Date: Tue, 02 Jul 2019 17:00:21 -0400

>>     Decode the data at CODING->src_object into CODING->dst_object.
>>     CODING->src_object is a buffer, a string, or nil.
>>     CODING->dst_object is a buffer.
>>  
>>     If CODING->src_object is a buffer, it must be the current buffer.
>>     In this case, if CODING->src_pos is positive, it is a position of
>>     the source text in the buffer, otherwise, the source text is in the
>>     gap area of the buffer, and CODING->src_pos specifies the offset of
>>     the text from GPT (which must be the same as PT).  If this is the
>>     same buffer as CODING->dst_object, CODING->src_pos must be
>>     negative.
>>  [...]
>>     The decoded data is inserted at the current point of the buffer
>>     CODING->dst_object.
>> 
>> but this doesn't say if the bytes are to be found originally at the
>> beginning of the gap or its end, nor whether they finish at the beginning or
>> the end, nor what happens in the middle and why it's been designed this way.
>
> It says that (a) CODING->src_pos is the negative of the offset from
> GPT of where the bytes are in the gap

Yes, I think this is actually wrong.
E.e. decode_coding_gap does:

    coding->src_chars = chars;
    [...]
    coding->src_pos = -chars;

so nowhere does it account for unspecified number of bytes between the
beginning of the gap and the beginning of the source text.
Here, `src_pos` is the offset from the end of the gap.

> (they don't have to be "at the
> end", AFAIU, just not "at the beginning");

Oh, indeed, src_pos doesn't need to start as the negation of src_chars. 

> As for why this was designed like that -- where else did you see
> comments in Emacs that answer this kind of questions?

There are such design comments at various places.
Usually added after the fact, sometimes added as part of
a commit-reversal to make sure someone else doesn't fall into the same
trap ;-)

>> Is the patch below correct?
> I think it describes conditions that don't need to exist.

How 'bout this new version.


        Stefan


diff --git a/src/coding.c b/src/coding.c
index 59589caee6..fd7e7aca0f 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -7323,10 +7323,15 @@ produce_annotation (struct coding_system *coding, ptrdiff_t pos)
    If CODING->src_object is a buffer, it must be the current buffer.
    In this case, if CODING->src_pos is positive, it is a position of
    the source text in the buffer, otherwise, the source text is in the
-   gap area of the buffer, and CODING->src_pos specifies the offset of
-   the text from GPT (which must be the same as PT).  If this is the
-   same buffer as CODING->dst_object, CODING->src_pos must be
-   negative.
+   gap area of the buffer, and CODING->src_pos specifies the
+   offset of the text from the end of the gap (which must be at PT).
+   If this is the same buffer as CODING->dst_object, CODING->src_pos must
+   be negative.
+
+   When the text is taken from the gap, it can't be at the beginning of
+   the gap so that we can produce the decoded text at the beginning of
+   the gap: this way, as the output grows, the input shrinks, so we only
+   need to allocate enough space for `max(IN, OUT)` instead of `IN + OUT`.
 
    If CODING->src_object is a string, CODING->src_pos is an index to
    that string.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.