bug#20498: 25.0.50; PATCH: break potential infinite loop in (line-move-to-column)

[Dima Kogan]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Dima Kogan <dima@secretsauce.net>
>> Date: Sun, 03 May 2015 12:32:33 -0700
>> 
>> (line-move-to-column) has a loop that can become infinite:
>> 
>>   (while (and ...)
>>     (goto-char (previous-char-property-change (point) line-beg)))
>> 
>> If (= (point) line-beg) then the (goto-char) does nothing, and the
>> condition in the while never changes.
>
> The full fragment is this:
>
>       (let ((line-beg (line-beginning-position)))
>         (while (and (not (bolp)) (invisible-p (1- (point))))
>           (goto-char (previous-char-property-change (point) line-beg))))))))
>
> So if (= (point) line-beg), then 'bolp' will return t, and we will
> break from the loop.  Am I missing something?
>
>> I'm seeing this in the wild with ERC and erc-fill-mode disabled.
>
> Can you dig deeper into the problem, and tell the details about this
> infloop?

Hi. Thank you very much for double-checking. It indeed looks like (bolp)
should cover this case. It doesn't however. When ERC misbehaves in this
way I observe both

 (= (line-beginning-position) (point))  --> t
 (bolp)                                 --> nil

This sounds wrong. Looking at the buffer with my eyes, the point is not
at the beginning of the line, so it LOOKS like the (bolp) result is
correct.

The docs for (line-beginning-position) state that this function respects
field boundaries. If I ignore those explicitly then I see the correct
behavior:

 (let ((inhibit-field-text-motion t))
       (line-beginning-position))   ---> correct start of line

So the correct patch would replace (line-beginning-position) to the
above expression. This is trivial, and I'm not actually attaching a
patch. If you want me to, tell me.

(line-beginning-position) is used in quite a few places in emacs, and
I'm wondering if in many of those uses the intent is to ignore fields.