Subject: Re: bug#36431: Crash in marker.c:337

From: Eli Zaretskii <[email-masked]>
Date: Wed, 03 Jul 2019 19:33:22 +0300

> From: Stefan Monnier <[email-masked]>
> Cc: [email-masked],  [email-masked]
> Date: Wed, 03 Jul 2019 12:19:22 -0400
> 
> > (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.

My point was that the code uses the sign of one of them and the value
of the other.

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

The text could be interpreted as meaning that the end of the gap must
be at PT.  IOW, "which" could either allude to the gap or to the end
of the gap.

> > (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 assumed that you wanted to clarify the comment, so preserving old
text sounds like a non-goal ;-)

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

No, I meant the code there actually tests these conditions, and if
they are not true, it moves the gap as needed to make them true.

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

I don't think it's true.  CODING->src_pos must be negative if the
source text is in the gap, period.

> >> +   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 ;-)  ]

Not in what I wrote, AFAICT.