bug#40000: 27.0.60; next-single-char-property-change hangs on bad argument

[Eli Zaretskii]

> From: Federico Tedin <federicotedin@gmail.com>
> Cc: casouri@gmail.com,  40000@debbugs.gnu.org
> Date: Tue, 14 Apr 2020 23:05:35 +0200
> 
> > Then we'd need to clearly document the behavior in each case, I guess.
> 
> Ok, I've taken another shot at it. I updated the function's
> documentation to reflect what happens with LIMIT when working on a
> string and on a buffer, hopefully it's clear enough.

Thanks.  I have a few minor comments:

> Subject: [PATCH] Prevent hanging in next-single-char-property-change
> 
> * src/textprop.c (Fnext_single_char_property_change): Prevent Emacs
> from hanging when the value of LIMIT is greater than the buffer's end
> position. (Bug#40000)

What you used as the description of the change is actually the
explanation why the change was made.  The description of the change
would be something like this:

  * src/textprop.c (Fnext_single_char_property_change): Clarify in
  the doc string the behavior when LIMIT is past the end of OBJECT.
  Stop the search when position gets to end of buffer, for when LIMIT
  is beyond that.  (Bug#40000)

The explanation should ideally be in the comments to the code.  In
this case, I'm not even sure we need an explanation, since there's a
reference to the bug report, and the explanation and the fix are quite
simple.

> -In a string, scan runs to the end of the string, unless LIMIT is non-nil.
> -In a buffer, if LIMIT is nil or omitted, it runs to (point-max), and the
> -value cannot exceed that.
> -If the optional fourth argument LIMIT is non-nil, don't search
> -past position LIMIT; return LIMIT if nothing is found before LIMIT.
> +In a string, scan runs to the end of the string, unless LIMIT is
> +non-nil, in which case its value is returned if nothing is found
> +before it.
> 
> -The property values are compared with `eq'.
> -If the property is constant all the way to the end of OBJECT, return the
> -last valid position in OBJECT.  */)
> +In a buffer, if LIMIT is nil or omitted, it runs to (point-max).  If
> +LIMIT is non-nil, scan does not go past it, and the smallest of
> +(point-max) and LIMIT is returned.
> +
> +The property values are compared with `eq'.  */)

Try to avoid passive tense in documentation, it makes the text longer
and sometimes harder to understand.  Here's how I'd rephrase this:

  In a string, scan runs to the end of the string, unless LIMIT is non-nil.
  In a buffer, scan runs to end of buffer, unless LIMIT is non-nil.
  If the optional fourth argument LIMIT is non-nil, don't search
  past position LIMIT; return LIMIT if nothing is found before LIMIT.
  However, if OBJECT is a buffer and LIMIT is beyond the end of the
  buffer, this function returns `point-max', not LIMIT.

  The property values are compared with `eq'.

> +         if (XFIXNAT (position) >= ZV)
> +           {
> +             XSETFASTINT (position, ZV);
> +             break;
> +           }

Here we expect 'position' to have the value of ZV already, right.
Then there's no need to use XSETFASTINT; if you want to make sure we
never get here with value >= ZV, you can add an assertion.