GNU bug report logs - #8254
race condition in dired.c's scmp function

Previous Next

Package: emacs;

Reported by: Paul Eggert <eggert <at> cs.ucla.edu>

Date: Tue, 15 Mar 2011 06:17:02 UTC

Severity: normal

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

Full log


Message #32 received at 8254 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Juanma Barranquero <lekktu <at> gmail.com>, 8254 <at> debbugs.gnu.org
Subject: Re: bug#8254: race condition in dired.c's scmp function
Date: Tue, 15 Mar 2011 14:27:51 -0700
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  <eggert <at> cs.ucla.edu>
 
+	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)





This bug report was last modified 14 years and 72 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.