From unknown Mon Jun 23 05:56:01 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#8254 <8254@debbugs.gnu.org> To: bug#8254 <8254@debbugs.gnu.org> Subject: Status: race condition in dired.c's scmp function Reply-To: bug#8254 <8254@debbugs.gnu.org> Date: Mon, 23 Jun 2025 12:56:01 +0000 retitle 8254 race condition in dired.c's scmp function reassign 8254 emacs submitter 8254 Paul Eggert severity 8254 normal thanks From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 02:16:43 2011 Received: (at submit) by debbugs.gnu.org; 15 Mar 2011 06:16:43 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzNYg-0006kb-CP for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:43 -0400 Received: from eggs.gnu.org ([140.186.70.92]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzNYe-0006kQ-FP for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzNYY-0008Oj-5f for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:35 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.1 Received: from lists.gnu.org ([199.232.76.165]:38955) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzNYX-0008Of-WF for submit@debbugs.gnu.org; Tue, 15 Mar 2011 02:16:34 -0400 Received: from [140.186.70.92] (port=60487 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzNYW-0003Yq-Ms for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:16:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzNYV-0008OE-38 for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:16:32 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]:57996) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzNYU-0008Ny-Pc for bug-gnu-emacs@gnu.org; Tue, 15 Mar 2011 02:16:31 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 7E36A39E80E1 for ; Mon, 14 Mar 2011 23:16:28 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ShOYv8N2HBZa for ; Mon, 14 Mar 2011 23:16:27 -0700 (PDT) Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 752D239E8083 for ; Mon, 14 Mar 2011 23:16:27 -0700 (PDT) Message-ID: <4D7F043A.5070702@cs.ucla.edu> Date: Mon, 14 Mar 2011 23:16:26 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8 MIME-Version: 1.0 To: bug-gnu-emacs@gnu.org Subject: race condition in dired.c's scmp function Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 199.232.76.165 X-Spam-Score: -4.6 (----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -4.6 (----) The following code in the Emacs trunk src/dired.c's scmp function has undefined behavior: while (l && (DOWNCASE ((unsigned char) *s1++) == DOWNCASE ((unsigned char) *s2++))) l--; Because the DOWNCASE macro assigns to the global variables case_temp1 and case_temp2, (DOWNCASE (x) == DOWNCASE (y)) is not valid, as the assignments can collide and lead to a race condition. This bug was found by gcc -Wsequence-point (GCC 4.5.2), which warns: dired.c:799:7: error: operation on 'case_temp2' may be undefined [-Wsequence-point] dired.c:799:7: error: operation on 'case_temp1' may be undefined [-Wsequence-point] I plan to work around the problem with something like the following patch. ---- Fix a race condition diagnosed by gcc -Wsequence-point. The expression (DOWNCASE ((unsigned char) *s1++) == DOWNCASE ((unsigned char) *s2++)), found in dired.c's scmp function, had undefined behavior. * lisp.h (DOWNCASE_TABLE, UPCASE_TABLE, DOWNCASE, UPPERCASEP): (NOCASEP, LOWERCASEP, UPCASE, UPCASE1): Move from here ... * buffer.h: ... to here, because these macros use current_buffer, and the new implementation with inline functions needs to have current_buffer in scope now, rather than later when the macros are used. (downcase, upcase1): New static inline functions. (DOWNCASE, UPCASE1): Reimplement using these functions. This avoids undefined behavior in expressions like DOWNCASE (x) == DOWNCASE (y), which previously suffered from race conditions in accessing the global variables case_temp1 and case_temp2. * casetab.c (case_temp1, case_temp2): Remove; no longer needed. * lisp.h (case_temp1, case_temp2): Remove their decls. * character.h (ASCII_CHAR_P): Move from here ... * lisp.h: ... to here, so that the inline functions mentioned above can use them. === modified file 'src/buffer.h' --- src/buffer.h 2011-03-13 22:25:16 +0000 +++ src/buffer.h 2011-03-15 05:52:48 +0000 @@ -1026,4 +1026,47 @@ #define PER_BUFFER_VALUE(BUFFER, OFFSET) \ (*(Lisp_Object *)((OFFSET) + (char *) (BUFFER))) - + +/* Current buffer's map from characters to lower-case characters. */ + +#define DOWNCASE_TABLE BVAR (current_buffer, downcase_table) + +/* Current buffer's map from characters to upper-case characters. */ + +#define UPCASE_TABLE BVAR (current_buffer, upcase_table) + +/* Downcase a character, or make no change if that cannot be done. */ + +static inline EMACS_INT +downcase (int ch) +{ + Lisp_Object down = CHAR_TABLE_REF (DOWNCASE_TABLE, ch); + return NATNUMP (down) ? XFASTINT (down) : ch; +} +#define DOWNCASE(CH) downcase (CH) + +/* 1 if CH is upper case. */ + +#define UPPERCASEP(CH) (DOWNCASE (CH) != (CH)) + +/* 1 if CH is neither upper nor lower case. */ + +#define NOCASEP(CH) (UPCASE1 (CH) == (CH)) + +/* 1 if CH is lower case. */ + +#define LOWERCASEP(CH) (!UPPERCASEP (CH) && !NOCASEP(CH)) + +/* Upcase a character, or make no change if that cannot be done. */ + +#define UPCASE(CH) (!UPPERCASEP (CH) ? UPCASE1 (CH) : (CH)) + +/* Upcase a character known to be not upper case. */ + +static inline EMACS_INT +upcase1 (int ch) +{ + Lisp_Object up = CHAR_TABLE_REF (UPCASE_TABLE, ch); + return NATNUMP (up) ? XFASTINT (up) : ch; +} +#define UPCASE1(CH) upcase1 (CH) === modified file 'src/casetab.c' --- src/casetab.c 2011-02-16 15:02:50 +0000 +++ src/casetab.c 2011-03-15 04:01:35 +0000 @@ -28,11 +28,6 @@ Lisp_Object Vascii_downcase_table, Vascii_upcase_table; Lisp_Object Vascii_canon_table, Vascii_eqv_table; -/* Used as a temporary in DOWNCASE and other macros in lisp.h. No - need to mark it, since it is used only very temporarily. */ -int case_temp1; -Lisp_Object case_temp2; - static void set_canon (Lisp_Object case_table, Lisp_Object range, Lisp_Object elt); static void set_identity (Lisp_Object table, Lisp_Object c, Lisp_Object elt); static void shuffle (Lisp_Object table, Lisp_Object c, Lisp_Object elt); @@ -302,4 +297,3 @@ defsubr (&Sset_case_table); defsubr (&Sset_standard_case_table); } - === modified file 'src/character.h' --- src/character.h 2011-03-08 04:37:19 +0000 +++ src/character.h 2011-03-15 05:52:52 +0000 @@ -128,9 +128,6 @@ XSETCDR ((x), tmp); \ } while (0) -/* Nonzero iff C is an ASCII character. */ -#define ASCII_CHAR_P(c) ((unsigned) (c) < 0x80) - /* Nonzero iff C is a character of code less than 0x100. */ #define SINGLE_BYTE_CHAR_P(c) ((unsigned) (c) < 0x100) === modified file 'src/lisp.h' --- src/lisp.h 2011-03-15 01:32:33 +0000 +++ src/lisp.h 2011-03-15 04:23:51 +0000 @@ -841,6 +841,9 @@ #endif /* not __GNUC__ */ +/* Nonzero iff C is an ASCII character. */ +#define ASCII_CHAR_P(c) ((unsigned) (c) < 0x80) + /* Almost equivalent to Faref (CT, IDX) with optimization for ASCII characters. Do not check validity of CT. */ #define CHAR_TABLE_REF(CT, IDX) \ @@ -2041,50 +2044,6 @@ #define QUITP (!NILP (Vquit_flag) && NILP (Vinhibit_quit)) -/* Variables used locally in the following case handling macros. */ -extern int case_temp1; -extern Lisp_Object case_temp2; - -/* Current buffer's map from characters to lower-case characters. */ - -#define DOWNCASE_TABLE BVAR (current_buffer, downcase_table) - -/* Current buffer's map from characters to upper-case characters. */ - -#define UPCASE_TABLE BVAR (current_buffer, upcase_table) - -/* Downcase a character, or make no change if that cannot be done. */ - -#define DOWNCASE(CH) \ - ((case_temp1 = (CH), \ - case_temp2 = CHAR_TABLE_REF (DOWNCASE_TABLE, case_temp1), \ - NATNUMP (case_temp2)) \ - ? XFASTINT (case_temp2) : case_temp1) - -/* 1 if CH is upper case. */ - -#define UPPERCASEP(CH) (DOWNCASE (CH) != (CH)) - -/* 1 if CH is neither upper nor lower case. */ - -#define NOCASEP(CH) (UPCASE1 (CH) == (CH)) - -/* 1 if CH is lower case. */ - -#define LOWERCASEP(CH) (!UPPERCASEP (CH) && !NOCASEP(CH)) - -/* Upcase a character, or make no change if that cannot be done. */ - -#define UPCASE(CH) (!UPPERCASEP (CH) ? UPCASE1 (CH) : (CH)) - -/* Upcase a character known to be not upper case. */ - -#define UPCASE1(CH) \ - ((case_temp1 = (CH), \ - case_temp2 = CHAR_TABLE_REF (UPCASE_TABLE, case_temp1), \ - NATNUMP (case_temp2)) \ - ? XFASTINT (case_temp2) : case_temp1) - extern Lisp_Object Vascii_downcase_table, Vascii_upcase_table; extern Lisp_Object Vascii_canon_table, Vascii_eqv_table; From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 03:06:21 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 07:06:21 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzOKj-0000Id-8w for submit@debbugs.gnu.org; Tue, 15 Mar 2011 03:06:21 -0400 Received: from fencepost.gnu.org ([140.186.70.10]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzOKg-0000IP-Lf for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 03:06:19 -0400 Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1PzOKa-0000Dq-Jl; Tue, 15 Mar 2011 03:06:12 -0400 Date: Tue, 15 Mar 2011 03:06:12 -0400 Message-Id: From: Eli Zaretskii To: Paul Eggert In-reply-to: <4D7F043A.5070702@cs.ucla.edu> (message from Paul Eggert on Mon, 14 Mar 2011 23:16:26 -0700) Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> X-Spam-Score: -6.5 (------) X-Debbugs-Envelope-To: 8254 Cc: 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Reply-To: Eli Zaretskii List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.5 (------) > Date: Mon, 14 Mar 2011 23:16:26 -0700 > From: Paul Eggert > Cc: > > The following code in the Emacs trunk src/dired.c's scmp function has > undefined behavior: > > while (l > && (DOWNCASE ((unsigned char) *s1++) > == DOWNCASE ((unsigned char) *s2++))) > l--; > > Because the DOWNCASE macro assigns to the global variables case_temp1 > and case_temp2, (DOWNCASE (x) == DOWNCASE (y)) is not valid, as the > assignments can collide and lead to a race condition. > [...] > I plan to work around the problem with something like the following > patch. Whew! How about a much simpler fix: while (l && (c1 = DOWNCASE ((unsigned char) *s1++), c2 = DOWNCASE ((unsigned char) *s2++), c1 == c2)) l--; (with suitable declarations of c1 and c2)? Will that fix the undefined behavior? From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 03:31:59 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 07:31:59 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzOjX-0000sn-1X for submit@debbugs.gnu.org; Tue, 15 Mar 2011 03:31:59 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzOjV-0000sc-Hp for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 03:31:58 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 1664739E80F7; Tue, 15 Mar 2011 00:31:52 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pb6+dOBAK8Vm; Tue, 15 Mar 2011 00:31:51 -0700 (PDT) Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id AE6CB39E80DF; Tue, 15 Mar 2011 00:31:51 -0700 (PDT) Message-ID: <4D7F15E7.1020407@cs.ucla.edu> Date: Tue, 15 Mar 2011 00:31:51 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8 MIME-Version: 1.0 To: Eli Zaretskii Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Score: -2.9 (--) X-Debbugs-Envelope-To: 8254 Cc: 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.9 (--) On 03/15/2011 12:06 AM, Eli Zaretskii wrote: > How about a much simpler fix: > > while (l > && (c1 = DOWNCASE ((unsigned char) *s1++), > c2 = DOWNCASE ((unsigned char) *s2++), > c1 == c2)) > l--; > > (with suitable declarations of c1 and c2)? Will that fix the > undefined behavior? Yes. But surely it's better to fix the problem so that usage of DOWNCASE is less error-prone. Generally speaking, code is easier to read and contains fewer errors when function-like macros act like functions. From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 06:50:51 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 10:50:51 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzRpy-0005XT-Pl for submit@debbugs.gnu.org; Tue, 15 Mar 2011 06:50:50 -0400 Received: from fencepost.gnu.org ([140.186.70.10]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzRpw-0005XG-Ey for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 06:50:49 -0400 Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1PzRpr-0008QC-4m; Tue, 15 Mar 2011 06:50:43 -0400 Date: Tue, 15 Mar 2011 06:50:43 -0400 Message-Id: From: Eli Zaretskii To: Paul Eggert In-reply-to: <4D7F15E7.1020407@cs.ucla.edu> (message from Paul Eggert on Tue, 15 Mar 2011 00:31:51 -0700) Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> X-Spam-Score: -6.5 (------) X-Debbugs-Envelope-To: 8254 Cc: 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Reply-To: Eli Zaretskii List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.5 (------) > Date: Tue, 15 Mar 2011 00:31:51 -0700 > From: Paul Eggert > CC: 8254@debbugs.gnu.org > > On 03/15/2011 12:06 AM, Eli Zaretskii wrote: > > How about a much simpler fix: > > > > while (l > > && (c1 = DOWNCASE ((unsigned char) *s1++), > > c2 = DOWNCASE ((unsigned char) *s2++), > > c1 == c2)) > > l--; > > > > (with suitable declarations of c1 and c2)? Will that fix the > > undefined behavior? > > Yes. But surely it's better to fix the problem so that > usage of DOWNCASE is less error-prone. Generally speaking, > code is easier to read and contains fewer errors > when function-like macros act like functions. Maybe, but I wonder if there's a better solution even if we decide to make these macros functions: I don't like to have the same static function in every file that includes buffer.h, on platforms that don't support inline functions. Anyway, this is Stefan's and Chong's call. I just voiced my astonishment that such a simple problem needs such a jumbo change. From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 07:37:32 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 11:37:32 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzSZA-0007Ls-9A for submit@debbugs.gnu.org; Tue, 15 Mar 2011 07:37:32 -0400 Received: from mail-gy0-f172.google.com ([209.85.160.172]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzSZ8-0007Lh-Tv for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 07:37:31 -0400 Received: by gyf3 with SMTP id 3so207109gyf.3 for <8254@debbugs.gnu.org>; Tue, 15 Mar 2011 04:37:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=5EQBg6LkaQuzVTYN4OKLoC2e4vr/b4EZ1i+CgP0aP9w=; b=uLEct2pe6TknNGYp2yXBCGDIuNrkY179ub6+7C9dPscK9atPxZMpmiSdwWqD/EWC1F vunDR/FwwBzT43A/5FcGZMhtYSiw5vyjxFBu1Y0CJvdFlOBDg3LyfUw+bRmlBzRV4+uf ZelXX0qqmk2hrmKis8PF+av+/GRwSCMkh7RAQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=NY2NWh4cnqBJYxX0PlgiyByvDx56kRr0N/s0SmsZsNEfjL+pGQhXwBxfXT0zNxTqwe MwBYxDW66bWiNPOCyOET7q94df7C2oxpvZZ/yxB+3raxjVuR0QZfLOmZCPZIJjBHmNyc QPPXYC6Aww/JOHyfmFN8wv2ZBiWSVQJFNtjCg= Received: by 10.151.27.16 with SMTP id e16mr3640930ybj.356.1300189045076; Tue, 15 Mar 2011 04:37:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.147.34.11 with HTTP; Tue, 15 Mar 2011 04:36:45 -0700 (PDT) In-Reply-To: <4D7F15E7.1020407@cs.ucla.edu> References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> From: Juanma Barranquero Date: Tue, 15 Mar 2011 12:36:45 +0100 Message-ID: Subject: Re: bug#8254: race condition in dired.c's scmp function To: Paul Eggert Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -3.2 (---) X-Debbugs-Envelope-To: 8254 Cc: Eli Zaretskii , 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.2 (---) On Tue, Mar 15, 2011 at 08:31, Paul Eggert wrote: > Generally speaking, > code is easier to read and contains fewer errors > when function-like macros act like functions. That's true, but then there's adding complexity when it is not needed. In this case, it is hard to know whether it is needed or not. On one hand, the only suspicious use of DOWNCASE is the one you pointed out. On the other hand, DOWNCASE begat UPPERCASEP, who begat LOWERCASEP and UPCASE (and also ISUPPER in regex.c). Not to mention UPCASE1, which uses the same variables and begat UPCASE (with LOWERCASEP) and NOCASEP... Though most of these cases use ?: and && and not =3D=3D, it is certainly tedious to check every instance of these macros. A (perhaps stupid) idea: would it be possible to define -DENABLE-CHECKING alternate versions of DOWNCASE and UPCASE1 which do some additional checking for side effects? That would allow finding the possible bugs during development. =C2=A0 =C2=A0 Juanma From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 12:52:34 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 16:52:35 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzXU2-00064A-3n for submit@debbugs.gnu.org; Tue, 15 Mar 2011 12:52:34 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzXTz-00063x-As for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 12:52:32 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 90F1839E80FF; Tue, 15 Mar 2011 09:52:25 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Nqbr8rgPQklL; Tue, 15 Mar 2011 09:52:25 -0700 (PDT) Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 19CE239E80E1; Tue, 15 Mar 2011 09:52:25 -0700 (PDT) Message-ID: <4D7F9948.5020907@cs.ucla.edu> Date: Tue, 15 Mar 2011 09:52:24 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7 MIME-Version: 1.0 To: Juanma Barranquero Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 8254 Cc: Eli Zaretskii , 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.3 (---) On 03/15/2011 04:36 AM, Juanma Barranquero wrote: > there's adding complexity when it is not needed. The patch subtracts complexity in one place (by removing global variables) and adds it in another (by creating static inline functions). Whether the overall effect is to decrease complexity, or to increase it, is debatable. Either way, it's not much of a change in complexity. There are efforts underway to make Emacs multithreaded. If that happens, a change like this will be needed, as the existing code is obviously not thread-safe. I don't see any real downside to installing this change in the trunk now. > A (perhaps stupid) idea: would it be possible to define > -DENABLE-CHECKING alternate versions of DOWNCASE and UPCASE1 which do > some additional checking for side effects? I plan to implement that sort of suggestion, but in a different way, by adding an --enable-gcc-warnings option to 'configure', which will cause it to pass extra options to GCC to catch this sort of problem. This option is already in used in several other projects, such as GNU coreutils, and Emacs would benefit from it as well. The option will be disabled by default, though, so that the warnings don't surprise people who don't expect them. From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 12:53:54 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 16:53:54 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzXVK-000661-J5 for submit@debbugs.gnu.org; Tue, 15 Mar 2011 12:53:54 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzXVJ-00065q-Gi for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 12:53:53 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 1723539E80F7; Tue, 15 Mar 2011 09:53:48 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WIpp9jpwtw6l; Tue, 15 Mar 2011 09:53:47 -0700 (PDT) Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 670EE39E80E1; Tue, 15 Mar 2011 09:53:47 -0700 (PDT) Message-ID: <4D7F999B.7080205@cs.ucla.edu> Date: Tue, 15 Mar 2011 09:53:47 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7 MIME-Version: 1.0 To: Eli Zaretskii Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 8254 Cc: 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.3 (---) On 03/15/2011 03:50 AM, Eli Zaretskii wrote: > I wonder if there's a better solution even if we decide to > make these macros functions We could move these macros to a new include file, and have it be included only by .c files that need the macros. That would be easy to do; the only real cost is that of having another include file to worry about. > I don't like to have the same static function in every file that > includes buffer.h, on platforms that don't support inline functions. These days, it's routine for compilers to inline. For old fashioned compilers that don't inline, it's routine to optimize away static functions that are never used. So, from an optimization viewpoint, this problem is relatively unimportant. From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 12:58:47 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 16:58:47 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzXa2-0006E7-Hr for submit@debbugs.gnu.org; Tue, 15 Mar 2011 12:58:46 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzXa0-0006Dt-9J for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 12:58:44 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id CA5FE39E80F7; Tue, 15 Mar 2011 09:58:38 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RzsXfhrciNek; Tue, 15 Mar 2011 09:58:38 -0700 (PDT) Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 4CFEE39E80E1; Tue, 15 Mar 2011 09:58:38 -0700 (PDT) Message-ID: <4D7F9ABE.70204@cs.ucla.edu> Date: Tue, 15 Mar 2011 09:58:38 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7 MIME-Version: 1.0 To: Juanma Barranquero Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 8254 Cc: Eli Zaretskii , 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.3 (---) On 03/15/2011 04:36 AM, Juanma Barranquero wrote: > On the other hand, DOWNCASE begat UPPERCASEP, who begat LOWERCASEP and > UPCASE (and also ISUPPER in regex.c). Not to mention UPCASE1, which > uses the same variables and begat UPCASE (with LOWERCASEP) and > NOCASEP... Though most of these cases use ?: and&& and not ==, it is > certainly tedious to check every instance of these macros. I agree; and not only is it tedious, it's error-prone. It's better to fix the macros so that there's no need to check, as follows. While we're at it, we should simply get rid of the macros, by replacing every use of UPPERCASEP with uppercasep, etc. === modified file 'src/buffer.h' --- src/buffer.h 2011-03-15 07:04:00 +0000 +++ src/buffer.h 2011-03-15 07:28:00 +0000 @@ -1047,19 +1047,12 @@ /* 1 if CH is upper case. */ -#define UPPERCASEP(CH) (DOWNCASE (CH) != (CH)) - -/* 1 if CH is neither upper nor lower case. */ - -#define NOCASEP(CH) (UPCASE1 (CH) == (CH)) - -/* 1 if CH is lower case. */ - -#define LOWERCASEP(CH) (!UPPERCASEP (CH) && !NOCASEP(CH)) - -/* Upcase a character, or make no change if that cannot be done. */ - -#define UPCASE(CH) (!UPPERCASEP (CH) ? UPCASE1 (CH) : (CH)) +static inline int +uppercasep (int ch) +{ + return downcase (ch) != ch; +} +#define UPPERCASEP(CH) uppercasep (CH) /* Upcase a character known to be not upper case. */ @@ -1070,3 +1063,30 @@ return NATNUMP (up) ? XFASTINT (up) : ch; } #define UPCASE1(CH) upcase1 (CH) + +/* 1 if CH is neither upper nor lower case. */ + +static inline int +nocasep (int ch) +{ + return upcase1 (ch) == ch; +} +#define NOCASEP(CH) nocasep (CH) + +/* 1 if CH is lower case. */ + +static inline int +lowercasep (int ch) +{ + return !uppercasep (ch) && !nocasep (ch); +} +#define LOWERCASEP(CH) lowercasep (CH) + +/* Upcase a character, or make no change if that cannot be done. */ + +static inline EMACS_INT +upcase (int ch) +{ + return uppercasep (ch) ? ch : upcase1 (ch); +} +#define UPCASE(CH) upcase (CH) From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 15:13:55 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 19:13:55 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzZgo-0000oQ-Ir for submit@debbugs.gnu.org; Tue, 15 Mar 2011 15:13:54 -0400 Received: from ironport2-out.teksavvy.com ([206.248.154.183] helo=ironport2-out.pppoe.ca) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzZgn-0000oF-1F for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 15:13:53 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvsEABtXf01FxIRf/2dsb2JhbACmCHjCQ4ViBJVbg0U X-IronPort-AV: E=Sophos;i="4.63,190,1299474000"; d="scan'208";a="96654268" Received: from 69-196-132-95.dsl.teksavvy.com (HELO ceviche.home) ([69.196.132.95]) by ironport2-out.pppoe.ca with ESMTP/TLS/ADH-AES256-SHA; 15 Mar 2011 15:13:47 -0400 Received: by ceviche.home (Postfix, from userid 20848) id E262D660C9; Tue, 15 Mar 2011 15:13:46 -0400 (EDT) From: Stefan Monnier To: Paul Eggert Subject: Re: bug#8254: race condition in dired.c's scmp function Message-ID: References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> <4D7F9ABE.70204@cs.ucla.edu> Date: Tue, 15 Mar 2011 15:13:46 -0400 In-Reply-To: <4D7F9ABE.70204@cs.ucla.edu> (Paul Eggert's message of "Tue, 15 Mar 2011 09:58:38 -0700") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -2.1 (--) X-Debbugs-Envelope-To: 8254 Cc: Juanma Barranquero , 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.1 (--) > I agree; and not only is it tedious, it's error-prone. It's better > to fix the macros so that there's no need to check, as follows. While > we're at it, we should simply get rid of the macros, by replacing > every use of UPPERCASEP with uppercasep, etc. If we're turning those macros into functions, then I indeed think we should get rid of the macros. I would also welcome those functions being real one-liners, as in: static inline int uppercasep (int ch) { return downcase (ch) != ch; } Stefan From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 15 17:28:03 2011 Received: (at 8254) by debbugs.gnu.org; 15 Mar 2011 21:28:03 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Pzbmc-0003oQ-VB for submit@debbugs.gnu.org; Tue, 15 Mar 2011 17:28:03 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzbmZ-0003nw-9T for 8254@debbugs.gnu.org; Tue, 15 Mar 2011 17:28:01 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 52B0839E8109; Tue, 15 Mar 2011 14:27:53 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ER9S6STVbO9X; Tue, 15 Mar 2011 14:27:51 -0700 (PDT) Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id DE97A39E80F2; Tue, 15 Mar 2011 14:27:51 -0700 (PDT) Message-ID: <4D7FD9D7.3070307@cs.ucla.edu> Date: Tue, 15 Mar 2011 14:27:51 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7 MIME-Version: 1.0 To: Stefan Monnier Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> <4D7F9ABE.70204@cs.ucla.edu> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 8254 Cc: Juanma Barranquero , 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.3 (---) On 03/15/2011 12:13 PM, Stefan Monnier wrote: > If we're turning those macros into functions, then I indeed think we > should get rid of the macros. I would also welcome those functions > being real one-liners OK, thanks, then I'll add the following patch to my planned change: === modified file 'src/ChangeLog' --- src/ChangeLog 2011-03-15 18:53:29 +0000 +++ src/ChangeLog 2011-03-15 21:23:54 +0000 @@ -1,5 +1,16 @@ 2011-03-15 Paul Eggert + Use functions, not macros, for up- and down-casing (Bug#8254). + * buffer.h (DOWNCASE_TABLE, UPCASE_TABLE, DOWNCASE, UPPERCASEP): + (NOCASEP, LOWERCASEP, UPCASE, UPCASE1): Remove. All callers changed + to use the following functions instead of these macros. + (downcase): Adjust to lack of DOWNCASE_TABLE. Return int, not + EMACS_INT, since callers assume the returned value fits in int. + (upcase1): Likewise, for UPCASE_TABLE. + (uppercasep, lowercasep, upcase): New static inline functions. + * editfns.c (Fchar_equal): Remove no-longer-needed workaround for + the race-condition problem in the old DOWNCASE. + * regex.c (CHARSET_LOOKUP_RANGE_TABLE_RAW, POP_FAILURE_REG_OR_COUNT): Rename locals to avoid shadowing. (regex_compile, re_match_2_internal): Move locals to avoid shadowing. === modified file 'src/buffer.h' --- src/buffer.h 2011-03-15 07:04:00 +0000 +++ src/buffer.h 2011-03-15 21:14:06 +0000 @@ -1027,46 +1027,30 @@ #define PER_BUFFER_VALUE(BUFFER, OFFSET) \ (*(Lisp_Object *)((OFFSET) + (char *) (BUFFER))) -/* Current buffer's map from characters to lower-case characters. */ - -#define DOWNCASE_TABLE BVAR (current_buffer, downcase_table) - -/* Current buffer's map from characters to upper-case characters. */ - -#define UPCASE_TABLE BVAR (current_buffer, upcase_table) - -/* Downcase a character, or make no change if that cannot be done. */ - -static inline EMACS_INT -downcase (int ch) -{ - Lisp_Object down = CHAR_TABLE_REF (DOWNCASE_TABLE, ch); - return NATNUMP (down) ? XFASTINT (down) : ch; -} -#define DOWNCASE(CH) downcase (CH) - -/* 1 if CH is upper case. */ - -#define UPPERCASEP(CH) (DOWNCASE (CH) != (CH)) - -/* 1 if CH is neither upper nor lower case. */ - -#define NOCASEP(CH) (UPCASE1 (CH) == (CH)) - -/* 1 if CH is lower case. */ - -#define LOWERCASEP(CH) (!UPPERCASEP (CH) && !NOCASEP(CH)) - -/* Upcase a character, or make no change if that cannot be done. */ - -#define UPCASE(CH) (!UPPERCASEP (CH) ? UPCASE1 (CH) : (CH)) - -/* Upcase a character known to be not upper case. */ - -static inline EMACS_INT -upcase1 (int ch) -{ - Lisp_Object up = CHAR_TABLE_REF (UPCASE_TABLE, ch); - return NATNUMP (up) ? XFASTINT (up) : ch; -} -#define UPCASE1(CH) upcase1 (CH) +/* Downcase a character C, or make no change if that cannot be done. */ +static inline int +downcase (int c) +{ + Lisp_Object downcase_table = BVAR (current_buffer, downcase_table); + Lisp_Object down = CHAR_TABLE_REF (downcase_table, c); + return NATNUMP (down) ? XFASTINT (down) : c; +} + +/* 1 if C is upper case. */ +static inline int uppercasep (int c) { return downcase (c) != c; } + +/* Upcase a character C known to be not upper case. */ +static inline int +upcase1 (int c) +{ + Lisp_Object upcase_table = BVAR (current_buffer, upcase_table); + Lisp_Object up = CHAR_TABLE_REF (upcase_table, c); + return NATNUMP (up) ? XFASTINT (up) : c; +} + +/* 1 if C is lower case. */ +static inline int lowercasep (int c) +{ return !uppercasep (c) && upcase1 (c) != c; } + +/* Upcase a character C, or make no change if that cannot be done. */ +static inline int upcase (int c) { return uppercasep (c) ? c : upcase1 (c); } === modified file 'src/casefiddle.c' --- src/casefiddle.c 2011-03-15 17:18:02 +0000 +++ src/casefiddle.c 2011-03-15 21:14:06 +0000 @@ -64,13 +64,13 @@ multibyte = 1; if (! multibyte) MAKE_CHAR_MULTIBYTE (c1); - c = DOWNCASE (c1); + c = downcase (c1); if (inword) XSETFASTINT (obj, c | flags); else if (c == (XFASTINT (obj) & ~flagbits)) { if (! inword) - c = UPCASE1 (c1); + c = upcase1 (c1); if (! multibyte) MAKE_CHAR_UNIBYTE (c); XSETFASTINT (obj, c | flags); @@ -92,10 +92,10 @@ MAKE_CHAR_MULTIBYTE (c); c1 = c; if (inword && flag != CASE_CAPITALIZE_UP) - c = DOWNCASE (c); - else if (!UPPERCASEP (c) + c = downcase (c); + else if (!uppercasep (c) && (!inword || flag != CASE_CAPITALIZE_UP)) - c = UPCASE1 (c1); + c = upcase1 (c1); if ((int) flag >= (int) CASE_CAPITALIZE) inword = (SYNTAX (c) == Sword); if (c != c1) @@ -133,10 +133,10 @@ } c = STRING_CHAR_AND_LENGTH (SDATA (obj) + i_byte, len); if (inword && flag != CASE_CAPITALIZE_UP) - c = DOWNCASE (c); - else if (!UPPERCASEP (c) + c = downcase (c); + else if (!uppercasep (c) && (!inword || flag != CASE_CAPITALIZE_UP)) - c = UPCASE1 (c); + c = upcase1 (c); if ((int) flag >= (int) CASE_CAPITALIZE) inword = (SYNTAX (c) == Sword); o += CHAR_STRING (c, o); @@ -243,10 +243,10 @@ } c2 = c; if (inword && flag != CASE_CAPITALIZE_UP) - c = DOWNCASE (c); - else if (!UPPERCASEP (c) + c = downcase (c); + else if (!uppercasep (c) && (!inword || flag != CASE_CAPITALIZE_UP)) - c = UPCASE1 (c); + c = upcase1 (c); if ((int) flag >= (int) CASE_CAPITALIZE) inword = ((SYNTAX (c) == Sword) && (inword || !syntax_prefix_flag_p (c))); === modified file 'src/dired.c' --- src/dired.c 2011-03-15 18:08:06 +0000 +++ src/dired.c 2011-03-15 21:14:06 +0000 @@ -790,8 +790,8 @@ if (completion_ignore_case) { while (l - && (DOWNCASE ((unsigned char) *s1++) - == DOWNCASE ((unsigned char) *s2++))) + && (downcase ((unsigned char) *s1++) + == downcase ((unsigned char) *s2++))) l--; } else === modified file 'src/editfns.c' --- src/editfns.c 2011-03-13 06:27:18 +0000 +++ src/editfns.c 2011-03-15 21:23:02 +0000 @@ -1374,7 +1374,7 @@ memcpy (r, p, q - p); r[q - p] = 0; strcat (r, SSDATA (login)); - r[q - p] = UPCASE ((unsigned char) r[q - p]); + r[q - p] = upcase ((unsigned char) r[q - p]); strcat (r, q + 1); full = build_string (r); } @@ -4213,7 +4213,7 @@ { int i1, i2; /* Check they're chars, not just integers, otherwise we could get array - bounds violations in DOWNCASE. */ + bounds violations in downcase. */ CHECK_CHARACTER (c1); CHECK_CHARACTER (c2); @@ -4222,9 +4222,6 @@ if (NILP (BVAR (current_buffer, case_fold_search))) return Qnil; - /* Do these in separate statements, - then compare the variables. - because of the way DOWNCASE uses temp variables. */ i1 = XFASTINT (c1); if (NILP (BVAR (current_buffer, enable_multibyte_characters)) && ! ASCII_CHAR_P (i1)) @@ -4237,9 +4234,7 @@ { MAKE_CHAR_MULTIBYTE (i2); } - i1 = DOWNCASE (i1); - i2 = DOWNCASE (i2); - return (i1 == i2 ? Qt : Qnil); + return (downcase (i1) == downcase (i2) ? Qt : Qnil); } /* Transpose the markers in two regions of the current buffer, and === modified file 'src/fileio.c' --- src/fileio.c 2011-03-15 03:17:20 +0000 +++ src/fileio.c 2011-03-15 21:14:06 +0000 @@ -178,7 +178,7 @@ str = SSDATA (errstring); c = STRING_CHAR ((unsigned char *) str); - Faset (errstring, make_number (0), make_number (DOWNCASE (c))); + Faset (errstring, make_number (0), make_number (downcase (c))); } xsignal (Qfile_error, === modified file 'src/keyboard.c' --- src/keyboard.c 2011-03-15 17:13:02 +0000 +++ src/keyboard.c 2011-03-15 21:14:06 +0000 @@ -9836,7 +9836,7 @@ && /* indec.start >= t && fkey.start >= t && */ keytran.start >= t && INTEGERP (key) && ((CHARACTERP (make_number (XINT (key) & ~CHAR_MODIFIER_MASK)) - && UPPERCASEP (XINT (key) & ~CHAR_MODIFIER_MASK)) + && uppercasep (XINT (key) & ~CHAR_MODIFIER_MASK)) || (XINT (key) & shift_modifier))) { Lisp_Object new_key; @@ -9847,7 +9847,7 @@ if (XINT (key) & shift_modifier) XSETINT (new_key, XINT (key) & ~shift_modifier); else - XSETINT (new_key, (DOWNCASE (XINT (key) & ~CHAR_MODIFIER_MASK) + XSETINT (new_key, (downcase (XINT (key) & ~CHAR_MODIFIER_MASK) | (XINT (key) & CHAR_MODIFIER_MASK))); /* We have to do this unconditionally, regardless of whether @@ -9875,13 +9875,13 @@ || (INTEGERP (key) && (KEY_TO_CHAR (key) < XCHAR_TABLE (BVAR (current_buffer, downcase_table))->size) - && UPPERCASEP (KEY_TO_CHAR (key)))) + && uppercasep (KEY_TO_CHAR (key)))) { Lisp_Object new_key = (modifiers & shift_modifier ? apply_modifiers (modifiers & ~shift_modifier, XCAR (breakdown)) - : make_number (DOWNCASE (KEY_TO_CHAR (key)) | modifiers)); + : make_number (downcase (KEY_TO_CHAR (key)) | modifiers)); original_uppercase = key; original_uppercase_position = t - 1; === modified file 'src/process.c' --- src/process.c 2011-03-14 22:49:41 +0000 +++ src/process.c 2011-03-15 21:14:06 +0000 @@ -495,7 +495,7 @@ string = (code_convert_string_norecord (string, Vlocale_coding_system, 0)); c1 = STRING_CHAR (SDATA (string)); - c2 = DOWNCASE (c1); + c2 = downcase (c1); if (c1 != c2) Faset (string, make_number (0), make_number (c2)); } === modified file 'src/regex.c' --- src/regex.c 2011-03-15 18:53:29 +0000 +++ src/regex.c 2011-03-15 21:14:06 +0000 @@ -340,7 +340,7 @@ || ((c) >= 'A' && (c) <= 'Z')) \ : SYNTAX (c) == Sword) -# define ISLOWER(c) (LOWERCASEP (c)) +# define ISLOWER(c) lowercasep (c) # define ISPUNCT(c) (IS_REAL_ASCII (c) \ ? ((c) > ' ' && (c) < 0177 \ @@ -351,7 +351,7 @@ # define ISSPACE(c) (SYNTAX (c) == Swhitespace) -# define ISUPPER(c) (UPPERCASEP (c)) +# define ISUPPER(c) uppercasep (c) # define ISWORD(c) (SYNTAX (c) == Sword) === modified file 'src/search.c' --- src/search.c 2011-03-15 18:13:15 +0000 +++ src/search.c 2011-03-15 21:14:06 +0000 @@ -2469,7 +2469,7 @@ else FETCH_STRING_CHAR_AS_MULTIBYTE_ADVANCE (c, string, pos, pos_byte); - if (LOWERCASEP (c)) + if (lowercasep (c)) { /* Cannot be all caps if any original char is lower case */ @@ -2479,7 +2479,7 @@ else some_multiletter_word = 1; } - else if (UPPERCASEP (c)) + else if (uppercasep (c)) { some_uppercase = 1; if (SYNTAX (prevc) != Sword) From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 16 09:19:27 2011 Received: (at 8254) by debbugs.gnu.org; 16 Mar 2011 13:19:27 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzqdL-0008Pv-Ct for submit@debbugs.gnu.org; Wed, 16 Mar 2011 09:19:27 -0400 Received: from fencepost.gnu.org ([140.186.70.10]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzqdK-0008Pl-DF for 8254@debbugs.gnu.org; Wed, 16 Mar 2011 09:19:26 -0400 Received: from rms by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1PzqdE-0003tp-Ol; Wed, 16 Mar 2011 09:19:20 -0400 Date: Wed, 16 Mar 2011 09:19:20 -0400 Message-Id: Content-Type: text/plain; charset=ISO-8859-15 From: Richard Stallman To: Paul Eggert In-reply-to: <4D7F9ABE.70204@cs.ucla.edu> (message from Paul Eggert on Tue, 15 Mar 2011 09:58:38 -0700) Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> <4D7F9ABE.70204@cs.ucla.edu> X-Spam-Score: -6.6 (------) X-Debbugs-Envelope-To: 8254 Cc: lekktu@gmail.com, 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Reply-To: rms@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.6 (------) I agree; and not only is it tedious, it's error-prone. It's better to fix the macros so that there's no need to check, as follows. While we're at it, we should simply get rid of the macros, by replacing every use of UPPERCASEP with uppercasep, etc. I see that uppercasep uses inline. That works fine in GCC, but does it work in all compilers anyone wants to use? If not, we should leave them as macros. These are used in some loops so their speed makes a difference. -- Dr Richard Stallman President, Free Software Foundation 51 Franklin St Boston MA 02110 USA www.fsf.org, www.gnu.org From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 16 16:02:31 2011 Received: (at 8254) by debbugs.gnu.org; 16 Mar 2011 20:02:32 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzwvP-0001BN-47 for submit@debbugs.gnu.org; Wed, 16 Mar 2011 16:02:31 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PzwvL-0001B9-Sj for 8254@debbugs.gnu.org; Wed, 16 Mar 2011 16:02:28 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id EBF1039E810E; Wed, 16 Mar 2011 13:02:21 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KVOjMeiWp68y; Wed, 16 Mar 2011 13:02:21 -0700 (PDT) Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 504F339E80DB; Wed, 16 Mar 2011 13:02:21 -0700 (PDT) Message-ID: <4D81174C.9040007@cs.ucla.edu> Date: Wed, 16 Mar 2011 13:02:20 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7 MIME-Version: 1.0 To: rms@gnu.org Subject: Re: bug#8254: race condition in dired.c's scmp function References: <4D7F043A.5070702@cs.ucla.edu> <4D7F15E7.1020407@cs.ucla.edu> <4D7F9ABE.70204@cs.ucla.edu> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 8254 Cc: lekktu@gmail.com, 8254@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.3 (---) On 03/16/2011 06:19 AM, Richard Stallman wrote: > I see that uppercasep uses inline. That works fine in GCC, but does > it work in all compilers anyone wants to use? I expect so, yes. That is, for all compilers that anybody wants to use, I expect that typically the performance difference will be so small that it will be hard to measure and will not be worth worrying about. I attempted to simulate this by compiling without optimization, both without and with the change, and measured roughly a 3% performance degradation on a simple large test case (running emacs interactively to upcase an 80 MB text file that was mostly lower case). This is sort of the worst case, since it assumes no inlining. In the normal case, when optimization is enabled, I observed a 3% performance improvement (on the same benchmark) due to the change. These measurements are noisy and approximate, but they indicate that there's not much performance impact either way in practice. From debbugs-submit-bounces@debbugs.gnu.org Thu Mar 17 12:56:24 2011 Received: (at 8254-done) by debbugs.gnu.org; 17 Mar 2011 16:56:24 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q0GUq-00057A-8s for submit@debbugs.gnu.org; Thu, 17 Mar 2011 12:56:24 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q0GUo-00056z-AK for 8254-done@debbugs.gnu.org; Thu, 17 Mar 2011 12:56:22 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id EC6AB39E810F for <8254-done@debbugs.gnu.org>; Thu, 17 Mar 2011 09:56:16 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id h1seYNQtKhBT for <8254-done@debbugs.gnu.org>; Thu, 17 Mar 2011 09:56:16 -0700 (PDT) Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id A23C639E80DB for <8254-done@debbugs.gnu.org>; Thu, 17 Mar 2011 09:56:16 -0700 (PDT) Message-ID: <4D823D30.6080106@cs.ucla.edu> Date: Thu, 17 Mar 2011 09:56:16 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.15) Gecko/20110307 Fedora/3.1.9-0.39.b3pre.fc14 Thunderbird/3.1.9 MIME-Version: 1.0 To: 8254-done@debbugs.gnu.org Subject: race condition in dired.c's scmp function Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.3 (---) X-Debbugs-Envelope-To: 8254-done X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.3 (---) I've committed a patch for this in bzr 103679. From unknown Mon Jun 23 05:56:01 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Fri, 15 Apr 2011 11:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator