From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Merging bignum to master Date: Mon, 13 Aug 2018 15:58:14 -0700 Organization: UCLA Computer Science Department Message-ID: <611579fd-52f2-0104-ef82-a7a4a3929700@cs.ucla.edu> References: <877ekwu1mn.fsf@tromey.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------F171A64536C1B90545D8DC62" X-Trace: blaine.gmane.org 1534200985 22442 195.159.176.226 (13 Aug 2018 22:56:25 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 13 Aug 2018 22:56:25 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 Cc: emacs-devel@gnu.org To: Pip Cet , tom@tromey.com Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Aug 14 00:56:21 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fpLl6-0005jc-FW for ged-emacs-devel@m.gmane.org; Tue, 14 Aug 2018 00:56:20 +0200 Original-Received: from localhost ([::1]:41712 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fpLnD-0005V3-6H for ged-emacs-devel@m.gmane.org; Mon, 13 Aug 2018 18:58:31 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:32772) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fpLn4-0005Tw-F4 for emacs-devel@gnu.org; Mon, 13 Aug 2018 18:58:23 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fpLn1-0003mJ-AB for emacs-devel@gnu.org; Mon, 13 Aug 2018 18:58:22 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:47690) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fpLn0-0003m7-UC for emacs-devel@gnu.org; Mon, 13 Aug 2018 18:58:19 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id DB6D8160661; Mon, 13 Aug 2018 15:58:16 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 7T5wrF3pn6fm; Mon, 13 Aug 2018 15:58:15 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id E5036160662; Mon, 13 Aug 2018 15:58:15 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id pgkroRsCG7fe; Mon, 13 Aug 2018 15:58:15 -0700 (PDT) Original-Received: from [192.168.1.9] (unknown [47.154.30.119]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 82EB2160661; Mon, 13 Aug 2018 15:58:15 -0700 (PDT) In-Reply-To: Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:228504 Archived-At: This is a multi-part message in MIME format. --------------F171A64536C1B90545D8DC62 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Pip Cet wrote: > I notice that there are a few places where you use XFIXNUMPTR > directly, rather than make_mint_ptr. Is that intentional? I think now > would be a good time to fix it and make XFIXNUMPTR internal to lisp.h. It's intentional since those few places don't need the full power of make_mint_ptr (often the pointers are already aligned) and there might be trouble if storage allocation fails if the pointer is not aligned. Perhaps you're right and this is overkill; it is confusing at any rate. In the meantime I installed the attached first patch to fix a glitch I saw in this area while looking at the current XFIXNUMPTR uses. > Also, the doc comments for most_negative_fixnum and > most_positive_fixnum in data.c still need updating. Thanks, done in the second attached patch. --------------F171A64536C1B90545D8DC62 Content-Type: text/x-patch; name="0001-Fix-check-for-unsafe-watch-descriptor.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Fix-check-for-unsafe-watch-descriptor.patch" >From 76101698a770d389f22b547c331ec78473040c47 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 13 Aug 2018 15:45:17 -0700 Subject: [PATCH 1/2] Fix check for unsafe watch descriptor * src/lisp.h (make_pointer_integer_unsafe): New function. (make_pointer_integer): Use it. * src/gfilenotify.c (dir_monitor_callback): Omit redundant eassert. (Fgfile_add_watch): Signal an error instead of failing an assertion if the pointer does not work. --- src/gfilenotify.c | 7 +++---- src/lisp.h | 8 +++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/gfilenotify.c b/src/gfilenotify.c index 7eea2cfac1..798f308b31 100644 --- a/src/gfilenotify.c +++ b/src/gfilenotify.c @@ -77,7 +77,6 @@ dir_monitor_callback (GFileMonitor *monitor, /* Determine callback function. */ monitor_object = make_pointer_integer (monitor); - eassert (FIXNUMP (monitor_object)); watch_object = assq_no_quit (monitor_object, watch_list); if (CONSP (watch_object)) @@ -203,10 +202,10 @@ will be reported only in case of the `moved' event. */) if (! monitor) xsignal2 (Qfile_notify_error, build_string ("Cannot watch file"), file); - Lisp_Object watch_descriptor = make_pointer_integer (monitor); + Lisp_Object watch_descriptor = make_pointer_integer_unsafe (monitor); - /* Check the dicey assumption that make_pointer_integer is safe. */ - if (! FIXNUMP (watch_descriptor)) + if (! (FIXNUMP (watch_descriptor) + && XFIXNUMPTR (watch_descriptor) == monitor)) { g_object_unref (monitor); xsignal2 (Qfile_notify_error, build_string ("Unsupported file watcher"), diff --git a/src/lisp.h b/src/lisp.h index b7ef8dc63a..18d53537cc 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -1188,10 +1188,16 @@ XFIXNUMPTR (Lisp_Object a) return XUNTAG (a, Lisp_Int0, char); } +INLINE Lisp_Object +make_pointer_integer_unsafe (void *p) +{ + return TAG_PTR (Lisp_Int0, p); +} + INLINE Lisp_Object make_pointer_integer (void *p) { - Lisp_Object a = TAG_PTR (Lisp_Int0, p); + Lisp_Object a = make_pointer_integer_unsafe (p); eassert (FIXNUMP (a) && XFIXNUMPTR (a) == p); return a; } -- 2.17.1 --------------F171A64536C1B90545D8DC62 Content-Type: text/x-patch; name="0002-Update-doc-strings-for-fixnum-constants.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Update-doc-strings-for-fixnum-constants.patch" >From dc18a0917a5531ef3e1c9b4921bb4d8f317bc7a4 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 13 Aug 2018 15:55:06 -0700 Subject: [PATCH 2/2] Update doc strings for fixnum constants * src/data.c (most-positive-fixnum, most-negative-fixnum): Update doc strings in the light of fixnums. --- src/data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/data.c b/src/data.c index 7b8dd45c94..a1215b9d6b 100644 --- a/src/data.c +++ b/src/data.c @@ -4260,13 +4260,13 @@ syms_of_data (void) set_symbol_function (Qwholenump, XSYMBOL (Qnatnump)->u.s.function); DEFVAR_LISP ("most-positive-fixnum", Vmost_positive_fixnum, - doc: /* The largest value that is representable in a Lisp integer. + doc: /* The greatest integer that is represented efficiently. This variable cannot be set; trying to do so will signal an error. */); Vmost_positive_fixnum = make_fixnum (MOST_POSITIVE_FIXNUM); make_symbol_constant (intern_c_string ("most-positive-fixnum")); DEFVAR_LISP ("most-negative-fixnum", Vmost_negative_fixnum, - doc: /* The smallest value that is representable in a Lisp integer. + doc: /* The least integer that is represented efficiently. This variable cannot be set; trying to do so will signal an error. */); Vmost_negative_fixnum = make_fixnum (MOST_NEGATIVE_FIXNUM); make_symbol_constant (intern_c_string ("most-negative-fixnum")); -- 2.17.1 --------------F171A64536C1B90545D8DC62--