From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Federico Tedin Newsgroups: gmane.emacs.bugs Subject: bug#40000: 27.0.60; next-single-char-property-change hangs on bad argument Date: Sun, 03 May 2020 16:04:50 +0200 Message-ID: <877dxtglml.fsf@gmail.com> 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> <831rob92jt.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="16816"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) Cc: casouri@gmail.com, 40000@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun May 03 16:06:10 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 1jVFFy-0004HK-8w for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 03 May 2020 16:06:10 +0200 Original-Received: from localhost ([::1]:43486 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jVFFx-0005aV-BT for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 03 May 2020 10:06:09 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34274) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jVFFq-0005aP-Sf for bug-gnu-emacs@gnu.org; Sun, 03 May 2020 10:06:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45955) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jVFFq-00069z-Jv for bug-gnu-emacs@gnu.org; Sun, 03 May 2020 10:06:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jVFFq-0001bL-Et for bug-gnu-emacs@gnu.org; Sun, 03 May 2020 10:06:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Federico Tedin Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 03 May 2020 14:06:02 +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.15885147056084 (code B ref 40000); Sun, 03 May 2020 14:06:02 +0000 Original-Received: (at 40000) by debbugs.gnu.org; 3 May 2020 14:05:05 +0000 Original-Received: from localhost ([127.0.0.1]:57501 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jVFEu-0001a4-SC for submit@debbugs.gnu.org; Sun, 03 May 2020 10:05:05 -0400 Original-Received: from mail-wr1-f45.google.com ([209.85.221.45]:45889) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jVFEp-0001ZR-9j for 40000@debbugs.gnu.org; Sun, 03 May 2020 10:05:03 -0400 Original-Received: by mail-wr1-f45.google.com with SMTP id o27so12396452wra.12 for <40000@debbugs.gnu.org>; Sun, 03 May 2020 07:04:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=EDJHS/LI8+gAL75QJYoIENVGtSXbf5+vu7jLb0sh8nE=; b=Al/LI2OZeoD3QP4O9j2OmwqcG4ZNq+YhhcMNLrG5yMYHQZnLFPyxsm3VoCgCxB3byc D9xN3TgEssf1JK7j/YqgVM6Mmp9x5z0L0QoJAw1pT9332r1FXuykWchj4ovuBHqg/Ds5 6H6fYD9MNSFgS75syNFI70mEE3NchJdRUmJQ74NdMI/1m2dmd/V9qEPgWeGptj+PuiD7 el8fUTEGtpJytgwFUxpkRz8diLmxZ5XTA7pgZBVGWRT2FWzQE/k2FT/kTSVFTilw5x9k M4BP/6XBPoTBJlI84vhymFvR+XLJAx+0ltNA1duPH1tsb6vS5MlfHHBdDGOla+BJM8FK e3gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=EDJHS/LI8+gAL75QJYoIENVGtSXbf5+vu7jLb0sh8nE=; b=jJpE5B+WkfAOSZ+kTF31XJOX8NfZGSYSscQeTfzGAmfkWYhDwiZ6w0NNXqtjKr0lPY lY73aFGmPKRoQBiGYEqOgLRn+VIvGVct7jcnAJR581neQKB/0z0fBKewvebWC0zGnbg2 ZSeF1EBDZMUWwrm7GHdCS4zchv6fLNW7KlM5aVKgFCY2u3wKXXuNc2OXT+ScPIStgK4/ EYxbQ+oCxU6ih9LLShCT0he4c745MP0gcqBHMwFjF2XoILmYSLrEskGgumHnVIBAnSco +AiYspVpd0DbffGFfFK1QP9CZpF/qbXz5ziV85+ALb6An+++hQmppIFzmce3uTfLjDPm 8EeQ== X-Gm-Message-State: AGi0Pua3BUL5+OtMjO0asHAMgf8y5wnCQtgIdhO+ouQ+yGlonEccZe4o cTIY1xfJa6/ztT4wq7AsO2m9uI6kM/k= X-Google-Smtp-Source: APiQypIDzgbS6eyz7gmhmGdvbqHDxcrW4WUdPJwP4oo8YRhREeiOifnrMlzdcKuq8RJZsUAUnzbaUg== X-Received: by 2002:adf:e58d:: with SMTP id l13mr15028155wrm.187.1588514692728; Sun, 03 May 2020 07:04:52 -0700 (PDT) Original-Received: from lead ([2a02:8109:8ac0:2ff0:f846:e136:aa78:2f03]) by smtp.gmail.com with ESMTPSA id h2sm8871807wmf.34.2020.05.03.07.04.51 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Sun, 03 May 2020 07:04:52 -0700 (PDT) In-Reply-To: <831rob92jt.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 25 Apr 2020 15:23:50 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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:179583 Archived-At: --=-=-= Content-Type: text/plain Hey Eli, thanks for the detailed feedback. > 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. Aah OK, that makes sense. I agree that the bug number should be enough if someone wants to find an explanation for the changes; I've changed the commit message to the one you proposed. >> -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. Good point, for some reason I am biased towards using the passive tense. I've changed the documentation string. > 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. Aah right, because `next-char-property-change' never returns values larger than `point-max'. So in the last iteration, `position' will be `ZV'. I don't feel like it's necessary to add an assertion though, unless the code inside the loop somehow was modified to be longer/more complex. I'm attaching an updated patch. - Fede --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Prevent-hanging-in-next-single-char-property-change.patch >From 6adaeec2a5681bb290a3a70968952780146abf66 Mon Sep 17 00:00:00 2001 From: Federico Tedin Date: Sun, 3 May 2020 15:47:56 +0200 Subject: [PATCH] Prevent hanging in next-single-char-property-change * 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) --- src/textprop.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/textprop.c b/src/textprop.c index 960dba3f8d..0876badc87 100644 --- a/src/textprop.c +++ b/src/textprop.c @@ -766,14 +766,13 @@ DEFUN ("next-single-char-property-change", Fnext_single_char_property_change, If OBJECT is a string, POSITION is a 0-based index into it. 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. +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 the property is constant all the way to the end of OBJECT, return the -last valid position in OBJECT. */) +The property values are compared with `eq'. */) (Lisp_Object position, Lisp_Object prop, Lisp_Object object, Lisp_Object limit) { if (STRINGP (object)) @@ -832,6 +831,9 @@ DEFUN ("next-single-char-property-change", Fnext_single_char_property_change, value = Fget_char_property (position, prop, object); if (!EQ (value, initial_value)) break; + + if (XFIXNAT (position) >= ZV) + break; } position = unbind_to (count, position); -- 2.17.1 --=-=-=--