This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Make regexec thread-safe (BZ #934)


Hi!

regexec is not listed in POSIX thread-safety exemption table and furthermore
http://www.opengroup.org/onlinepubs/009695399/functions/regcomp.html
has:
"The interface is defined so that the matched substrings rm_sp and rm_ep
are in a separate regmatch_t structure instead
of in regex_t. This allows a single compiled RE to be used
simultaneously in several contexts; in main() and a signal
handler, perhaps, or in multiple threads of lightweight processes."

I don't think it is too common thing to do to use one regex_t
simultaneously, so I think one big lock in re_dfa_t is enough.

Because of:
"If the preg argument to regexec() or regfree() is not a compiled
regular expression returned by regcomp(), the result is
undefined. A preg is no longer treated as a compiled regular expression
after it is given to regfree()."
I think we need no locking in regcomp nor regfree, as they can't be called
simultaneously on the same regex_t, nor simultaneously with any regexec
calls on the same regex_t.

2005-05-06  Jakub Jelinek  <jakub@redhat.com>

	[BZ #934]
	* posix/regex_internal: Include bits/libc-lock.h or define dummy
	__libc_lock_* macros if not _LIBC.
	(struct re_dfa_t): Add lock.
	* posix/regcomp.c (re_compile_internal): Add __libc_lock_init.
	* posix/regexec.c (regexec, re_search_stub): Add locking.

--- libc/posix/regex_internal.h.jj	2005-02-21 17:20:06.000000000 +0100
+++ libc/posix/regex_internal.h	2005-05-06 10:56:34.000000000 +0200
@@ -39,6 +39,14 @@
 #if defined HAVE_WCTYPE_H || defined _LIBC
 # include <wctype.h>
 #endif /* HAVE_WCTYPE_H || _LIBC */
+#if defined _LIBC
+# include <bits/libc-lock.h>
+#else
+# define __libc_lock_define(CLASS,NAME)
+# define __libc_lock_init(NAME) do { } while (0)
+# define __libc_lock_lock(NAME) do { } while (0)
+# define __libc_lock_unlock(NAME) do { } while (0)
+#endif
 
 /* In case that the system doesn't have isblank().  */
 #if !defined _LIBC && !defined HAVE_ISBLANK && !defined isblank
@@ -647,6 +655,7 @@ struct re_dfa_t
 #ifdef DEBUG
   char* re_str;
 #endif
+  __libc_lock_define (, lock)
 };
 
 #ifndef RE_NO_INTERNAL_PROTOTYPES
--- libc/posix/regcomp.c.jj	2005-04-13 21:29:19.000000000 +0200
+++ libc/posix/regcomp.c	2005-05-06 10:58:03.000000000 +0200
@@ -774,6 +774,8 @@ re_compile_internal (preg, pattern, leng
     }
   preg->used = sizeof (re_dfa_t);
 
+  __libc_lock_init (dfa->lock);
+
   err = init_dfa (dfa, length);
   if (BE (err != REG_NOERROR, 0))
     {
--- libc/posix/regexec.c.jj	2005-03-08 13:05:18.000000000 +0100
+++ libc/posix/regexec.c	2005-05-06 11:11:04.000000000 +0200
@@ -219,6 +219,7 @@ regexec (preg, string, nmatch, pmatch, e
 {
   reg_errcode_t err;
   int start, length;
+  re_dfa_t *dfa = (re_dfa_t *)preg->buffer;
 
   if (eflags & ~(REG_NOTBOL | REG_NOTEOL | REG_STARTEND))
     return REG_BADPAT;
@@ -233,12 +234,15 @@ regexec (preg, string, nmatch, pmatch, e
       start = 0;
       length = strlen (string);
     }
+
+  __libc_lock_lock (dfa->lock);
   if (preg->no_sub)
     err = re_search_internal (preg, string, length, start, length - start,
 			      length, 0, NULL, eflags);
   else
     err = re_search_internal (preg, string, length, start, length - start,
 			      length, nmatch, pmatch, eflags);
+  __libc_lock_unlock (dfa->lock);
   return err != REG_NOERROR;
 }
 
@@ -402,6 +406,7 @@ re_search_stub (bufp, string, length, st
   regmatch_t *pmatch;
   int nregs, rval;
   int eflags = 0;
+  re_dfa_t *dfa = (re_dfa_t *)bufp->buffer;
 
   /* Check for out-of-range.  */
   if (BE (start < 0 || start > length, 0))
@@ -411,6 +416,8 @@ re_search_stub (bufp, string, length, st
   else if (BE (start + range < 0, 0))
     range = -start;
 
+  __libc_lock_lock (dfa->lock);
+
   eflags |= (bufp->not_bol) ? REG_NOTBOL : 0;
   eflags |= (bufp->not_eol) ? REG_NOTEOL : 0;
 
@@ -439,7 +446,10 @@ re_search_stub (bufp, string, length, st
     nregs = bufp->re_nsub + 1;
   pmatch = re_malloc (regmatch_t, nregs);
   if (BE (pmatch == NULL, 0))
-    return -2;
+    {
+      __libc_lock_unlock (dfa->lock);
+      return -2;
+    }
 
   result = re_search_internal (bufp, string, length, start, range, stop,
 			       nregs, pmatch, eflags);
@@ -469,6 +479,7 @@ re_search_stub (bufp, string, length, st
 	rval = pmatch[0].rm_so;
     }
   re_free (pmatch);
+  __libc_lock_unlock (dfa->lock);
   return rval;
 }
 

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]