bug#36431: Crash in marker.c:337

[Stefan Monnier]

> (Note that it actually only uses the byte offset's numerical value.  I
> couldn't find any place where it actually uses the character offset in
> src_pos, it only checks its sign.)

The source is required to be unibyte, so src_pos and src_pos_byte have
to be equal at start and one of the two is redundant.

>> 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 ;-)
> Interesting.  Can you point me to a couple of such design comments?

Not off-hand, no, but I know I added such comments every once in a while.

>>     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).
>
> The "which must be at PT" part is ambiguous.  I suggest to say
> explicitly that the gap must be at PT

AFAICT that's exactly what it is saying, so I'm not sure what ambiguity
you're thinking of.

> (although decode-coding doesn't really blindly assume that, as you can
> see from its calls to move_gap_both).

[ FWIW, this part of the text is mostly preserved from the old text.  ]
I think the issue is that decode_coding's calls to move_gap_both *must*
be no-ops when the source is in the gap otherwise it'll destroy the
source-text.

>> +   If this is the same buffer as CODING->dst_object, CODING->src_pos must
>> +   be negative.
> I don't see the condition of the same buffer in the code.  What did I
> miss?

This one I don't know: I just preserved this part of the text.

>> +   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`.
>
> I think this should also tell that decoding in this setup takes bytes
> from encoded text at the end of the gap and inserts the decoded text
> starting at PT, which is the same as the beginning of the gap.

[ PT is both the beginning and the end of the gap ;-)  ]
OK, I'll clarify a bit, thanks.


        Stefan