Hi Guenter, I've reviewed your patch and have some comments included inline below.
From: "Guenter Knauf" <[EMAIL PROTECTED]> > Hi all, > the previous patch seems to break non-Apache connectors, so here's another patch which shouldnt break anything unless you set the HAVE_AP_PCRE. > > I would like to get this patch into CVS in order to use PCRE on NetWare where only the ap_ prefixed pcre functions are exported. I have tested that the patch below compiles fine with NetWare, Linux and Win32. > As a positive side effect on Win32 the binary is about 24 kb smaller. > Also this time tested that the isapi redirector still compiles fine. > > thanks, Guenter. > > http://www.gknw.com/test/pcre_patch2 > > ###################################################################### ######### > --- ./jk/native2/common/jk_uriEnv.c.orig 2004-02-24 12:30:10.000000000 +0100 > +++ ./jk/native2/common/jk_uriEnv.c 2004-02-26 00:12:57.000000000 +0100 > @@ -29,8 +29,16 @@ > #include "jk_registry.h" > > #ifdef HAS_PCRE > -#include "pcre.h" > -#include "pcreposix.h" > + #ifdef HAS_AP_PCRE > + #include "httpd.h" > + #undef regcomp > + #define regcomp ap_pregcomp regcomp and ap_pregcomp are not interchangeable like this. ap_pregcomp needs an apr_pool to be passed to it and it returns the regex_t. I think (apr_pool_t *)uriEnv->pool->_private is correct here (Henri?, Jean-Frederic?). I'm not a fan of undef/def functions like this. If the functions were interchangeable, I would have just created a new define like PREGCOMP that points to the correct function. > + #define REGEX_POOL apr_pool_t > + #else > + #include "pcre.h" > + #include "pcreposix.h" > + #define REGEX_POOL regex_t > + #endif > #endif Since Apache2 always comes with pcre whole define section would be better like this: #ifdef HAS_AP_PCRE #include "httpd.h" #else #ifdef HAS_PCRE #include "pcre.h" #include "pcreposix.h" #endif #endif This way Apache2 will not require the --with-pcre configure argument that also includes -lpcre -lpcreposix libs when linking (not needed for Apache2). Along with this change you need to change server/apache2/Makefile.in and Makefile.apx.in to remove @HAS_PCRE@ and @PCRE_LIBS@ and add -DHAS_AP_PCRE. > > /* return non-zero if pattern has any glob chars in it */ > @@ -73,7 +81,7 @@ > "uriEnv.parseName() parsing %s regexp\n", > name); > { > - regex_t *preg = (regex_t *)uriEnv->pool->calloc( env, uriEnv->pool, sizeof(regex_t)); > + REGEX_POOL *preg = (REGEX_POOL *)uriEnv->pool->calloc( env, uriEnv->pool, sizeof(regex_t)); Similar to my comments above, this calloc and casting to apr_pool_t is not correct for ap_pregcomp. > if (regcomp(preg, uriEnv->uri, REG_EXTENDED)) { > env->l->jkLog(env, env->l, JK_LOG_DEBUG, > "uriEnv.parseName() error compiling regexp %s\n", > @@ -138,7 +146,7 @@ > "uriEnv.parseName() parsing regexp %s\n", > uri); > { > - regex_t *preg = (regex_t *)uriEnv->pool->calloc( env, uriEnv->pool, sizeof(regex_t)); > + REGEX_POOL *preg = (REGEX_POOL *)uriEnv->pool->calloc( env, uriEnv->pool, sizeof(regex_t)); > if (regcomp(preg, uriEnv->uri, REG_EXTENDED)) { > env->l->jkLog(env, env->l, JK_LOG_DEBUG, > "uriEnv.parseName() error compiling regexp %s\n", > ###################################################################### ######### > --- ./jk/native2/common/jk_uriMap.c.orig 2004-02-24 12:30:10.000000000 +0100 > +++ ./jk/native2/common/jk_uriMap.c 2004-02-26 00:13:09.000000000 +0100 > @@ -35,8 +35,14 @@ > #include "jk_registry.h" > > #ifdef HAS_PCRE > -#include "pcre.h" > -#include "pcreposix.h" > + #ifdef HAS_AP_PCRE > + #include "httpd.h" > + #undef regexec > + #define regexec ap_regexec > + #else > + #include "pcre.h" > + #include "pcreposix.h" > + #endif > #endif Similar to above comments, I'd like to something like this instead: #ifdef HAS_AP_PCRE #include "httpd.h" #define REGEXEC ap_regexec #else #ifdef HAS_PCRE #include "pcre.h" #include "pcreposix.h" #define REGEXEC regexec #endif #endif > > static INLINE const char *jk2_findExtension(jk_env_t *env, const char *uri); > > > > -------------------------------------------------------------------- - > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
