* [PATCH] Make regexec thread-safe (BZ #934)
@ 2005-05-06 9:39 Jakub Jelinek
2005-05-06 23:34 ` Ulrich Drepper
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2005-05-06 9:39 UTC (permalink / raw)
To: Ulrich Drepper, Roland McGrath; +Cc: Glibc hackers
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Make regexec thread-safe (BZ #934)
2005-05-06 9:39 [PATCH] Make regexec thread-safe (BZ #934) Jakub Jelinek
@ 2005-05-06 23:34 ` Ulrich Drepper
0 siblings, 0 replies; 2+ messages in thread
From: Ulrich Drepper @ 2005-05-06 23:34 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Glibc hackers
[-- Attachment #1: Type: text/plain, Size: 866 bytes --]
Jakub Jelinek wrote:
> 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."
The latter is not in normative text and the former doesn't really mean
that this kind of concurrency is required. Using the same regex_t
concurrently is unspecified.
Having said this, it certainly is a useful and not too expensive
extension. I added the patch.
--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2005-05-06 23:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-05-06 9:39 [PATCH] Make regexec thread-safe (BZ #934) Jakub Jelinek
2005-05-06 23:34 ` Ulrich Drepper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).