From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#40000: 27.0.60; next-single-char-property-change hangs on bad argument Date: Sat, 25 Apr 2020 15:23:50 +0300 Message-ID: <831rob92jt.fsf@gnu.org> References: <87d08b333i.fsf@gmail.com> <83tv1niira.fsf@gnu.org> <878siz31ta.fsf@gmail.com> <83sgh7ihj2.fsf@gnu.org> <874ktn2yph.fsf@gmail.com> <83pncbidov.fsf@gnu.org> <87v9m1dbhc.fsf@gmail.com> Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="28219"; mail-complaints-to="usenet@ciao.gmane.io" Cc: casouri@gmail.com, 40000@debbugs.gnu.org To: Federico Tedin Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Apr 25 14:25:13 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jSJrt-0007DE-20 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 25 Apr 2020 14:25:13 +0200 Original-Received: from localhost ([::1]:36108 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jSJrs-0006vX-27 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 25 Apr 2020 08:25:12 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:40128) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jSJrj-0006vN-Cb for bug-gnu-emacs@gnu.org; Sat, 25 Apr 2020 08:25:03 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.90_1) (envelope-from ) id 1jSJri-00083D-BI for bug-gnu-emacs@gnu.org; Sat, 25 Apr 2020 08:25:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:47436) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jSJrh-00081t-UV for bug-gnu-emacs@gnu.org; Sat, 25 Apr 2020 08:25:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jSJrh-00072d-QR for bug-gnu-emacs@gnu.org; Sat, 25 Apr 2020 08:25:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 25 Apr 2020 12:25:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 40000 X-GNU-PR-Package: emacs Original-Received: via spool by 40000-submit@debbugs.gnu.org id=B40000.158781745026996 (code B ref 40000); Sat, 25 Apr 2020 12:25:01 +0000 Original-Received: (at 40000) by debbugs.gnu.org; 25 Apr 2020 12:24:10 +0000 Original-Received: from localhost ([127.0.0.1]:58980 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jSJqs-00071L-5z for submit@debbugs.gnu.org; Sat, 25 Apr 2020 08:24:10 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:36948) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jSJqq-000716-EW for 40000@debbugs.gnu.org; Sat, 25 Apr 2020 08:24:08 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:53828) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jSJql-0005pq-5c; Sat, 25 Apr 2020 08:24:03 -0400 Original-Received: from [176.228.60.248] (port=1381 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1jSJqj-0000VV-KS; Sat, 25 Apr 2020 08:24:02 -0400 In-Reply-To: <87v9m1dbhc.fsf@gmail.com> (message from Federico Tedin on Tue, 14 Apr 2020 23:05:35 +0200) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:178986 Archived-At: > From: Federico Tedin > 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.