You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rj...@apache.org on 2014/07/22 21:29:08 UTC
svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Author: rjung
Date: Tue Jul 22 19:29:08 2014
New Revision: 1612653
URL: http://svn.apache.org/r1612653
Log:
Clarify comment.
Modified:
httpd/httpd/trunk/server/util_pcre.c
Modified: httpd/httpd/trunk/server/util_pcre.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1612653&r1=1612652&r2=1612653&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util_pcre.c (original)
+++ httpd/httpd/trunk/server/util_pcre.c Tue Jul 22 19:29:08 2014
@@ -125,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t *
const char *errorptr;
int erroffset;
int errcode = 0;
- /* PCRE_DUPNAMES is only present in more recent versions of PCRE */
+ /* PCRE_DUPNAMES is only present since version 6.7 of PCRE */
#ifdef PCRE_DUPNAMES
int options = PCRE_DUPNAMES;
#else
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Posted by Marion & Christophe JAILLET <ch...@wanadoo.fr>.
+1
Le 22/07/2014 23:01, Rainer Jung a écrit :
> On 22.07.2014 22:20, Christophe JAILLET wrote:
>> Hi,
>>
>> shouldn't the #error just a few lines below be updated as well, to be
>> more explicit than "too old" ?
>
> You are right. But what about instead changing the configure pcre
> version test:
>
> Index: configure.in
> ===================================================================
> --- configure.in (revision 1611600)
> +++ configure.in (working copy)
> @@ -236,7 +236,9 @@
> fi
> case `$PCRE_CONFIG --version` in
> [[1-5].*])
> - AC_MSG_ERROR([Need at least pcre version 6.0])
> + AC_MSG_ERROR([Need at least pcre version 6.7])
> + [6.[0-6]*])
> + AC_MSG_ERROR([Need at least pcre version 6.7])
> ;;
> esac
> AC_MSG_NOTICE([Using external PCRE library from $PCRE_CONFIG])
>
> documenting the requirement PCRE >= 6.7 and dropping the check (and
> error message) for PCRE_DUPNAMES from server/util_pcre.c.
>
> I got the version number from looking and the old PCRE tags.
>
> Regards,
>
> Rainer
>
>> Le 22/07/2014 21:29, rjung@apache.org a écrit :
>>> Author: rjung
>>> Date: Tue Jul 22 19:29:08 2014
>>> New Revision: 1612653
>>>
>>> URL: http://svn.apache.org/r1612653
>>> Log:
>>> Clarify comment.
>>>
>>> Modified:
>>> httpd/httpd/trunk/server/util_pcre.c
>>>
>>> Modified: httpd/httpd/trunk/server/util_pcre.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1612653&r1=1612652&r2=1612653&view=diff
>>>
>>>
>>> ==============================================================================
>>>
>>>
>>> --- httpd/httpd/trunk/server/util_pcre.c (original)
>>> +++ httpd/httpd/trunk/server/util_pcre.c Tue Jul 22 19:29:08 2014
>>> @@ -125,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t *
>>> const char *errorptr;
>>> int erroffset;
>>> int errcode = 0;
>>> - /* PCRE_DUPNAMES is only present in more recent versions of
>>> PCRE */
>>> + /* PCRE_DUPNAMES is only present since version 6.7 of PCRE */
>>> #ifdef PCRE_DUPNAMES
>>> int options = PCRE_DUPNAMES;
>>> #else
>
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Posted by Guenter Knauf <fu...@apache.org>.
Hi Rainer,
On 23.07.2014 12:18, Rainer Jung wrote:
> Good point. But in this case even after dropping the check, it would
> mean the build would error out because PCRE_DUPNAMES isn't known. So
> then you would have to check the (new) info in the docs, that minimum
> PCRE version is 6.7.
I meant to keep this check (which you removed with 1612921) ...
> Of course an #error message would help because it is more explicit, but
> I think that's not how the code currently is structured in general. If
> there is a dependency for a library version, we don't check in the
> sources whether the right version is available.
not true - we do; f.e. from mod_ssl_ct.c:
#include "apr_version.h"
#if !APR_VERSION_AT_LEAST(1,5,0)
#error mod_ssl_ct requires APR 1.5.0 or later! (for apr_escape.h)
#endif
and:
#if OPENSSL_VERSION_NUMBER < 0x10002001L
#error "mod_ssl_ct requires OpenSSL 1.0.2-beta1 or later"
#endif
and I'm sure there are many more similar places ...
but I think the check should directly happen after pcre.h is included
and not in the middle of code; see 1612945.
Günter.
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 23.07.2014 02:25, Guenter Knauf wrote:
> Hi Rainer,
> On 22.07.2014 23:01, Rainer Jung wrote:
>> documenting the requirement PCRE >= 6.7 and dropping the check (and
>> error message) for PCRE_DUPNAMES from server/util_pcre.c.
> -1.
> Please think of non-configure builds;
> it doesnt hurt if the code errors out when the requirements do not met.
Good point. But in this case even after dropping the check, it would
mean the build would error out because PCRE_DUPNAMES isn't known. So
then you would have to check the (new) info in the docs, that minimum
PCRE version is 6.7.
Of course an #error message would help because it is more explicit, but
I think that's not how the code currently is structured in general. If
there is a dependency for a library version, we don't check in the
sources whether the right version is available.
Regards,
Rainer
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Posted by Guenter Knauf <fu...@apache.org>.
Hi Rainer,
On 22.07.2014 23:01, Rainer Jung wrote:
> documenting the requirement PCRE >= 6.7 and dropping the check (and
> error message) for PCRE_DUPNAMES from server/util_pcre.c.
-1.
Please think of non-configure builds;
it doesnt hurt if the code errors out when the requirements do not met.
Gün.
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 22.07.2014 22:20, Christophe JAILLET wrote:
> Hi,
>
> shouldn't the #error just a few lines below be updated as well, to be
> more explicit than "too old" ?
You are right. But what about instead changing the configure pcre
version test:
Index: configure.in
===================================================================
--- configure.in (revision 1611600)
+++ configure.in (working copy)
@@ -236,7 +236,9 @@
fi
case `$PCRE_CONFIG --version` in
[[1-5].*])
- AC_MSG_ERROR([Need at least pcre version 6.0])
+ AC_MSG_ERROR([Need at least pcre version 6.7])
+ [6.[0-6]*])
+ AC_MSG_ERROR([Need at least pcre version 6.7])
;;
esac
AC_MSG_NOTICE([Using external PCRE library from $PCRE_CONFIG])
documenting the requirement PCRE >= 6.7 and dropping the check (and
error message) for PCRE_DUPNAMES from server/util_pcre.c.
I got the version number from looking and the old PCRE tags.
Regards,
Rainer
> Le 22/07/2014 21:29, rjung@apache.org a écrit :
>> Author: rjung
>> Date: Tue Jul 22 19:29:08 2014
>> New Revision: 1612653
>>
>> URL: http://svn.apache.org/r1612653
>> Log:
>> Clarify comment.
>>
>> Modified:
>> httpd/httpd/trunk/server/util_pcre.c
>>
>> Modified: httpd/httpd/trunk/server/util_pcre.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1612653&r1=1612652&r2=1612653&view=diff
>>
>> ==============================================================================
>>
>> --- httpd/httpd/trunk/server/util_pcre.c (original)
>> +++ httpd/httpd/trunk/server/util_pcre.c Tue Jul 22 19:29:08 2014
>> @@ -125,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t *
>> const char *errorptr;
>> int erroffset;
>> int errcode = 0;
>> - /* PCRE_DUPNAMES is only present in more recent versions of PCRE */
>> + /* PCRE_DUPNAMES is only present since version 6.7 of PCRE */
>> #ifdef PCRE_DUPNAMES
>> int options = PCRE_DUPNAMES;
>> #else
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Posted by Christophe JAILLET <ch...@wanadoo.fr>.
Hi,
shouldn't the #error just a few lines below be updated as well, to be
more explicit than "too old" ?
CJ
Le 22/07/2014 21:29, rjung@apache.org a écrit :
> Author: rjung
> Date: Tue Jul 22 19:29:08 2014
> New Revision: 1612653
>
> URL: http://svn.apache.org/r1612653
> Log:
> Clarify comment.
>
> Modified:
> httpd/httpd/trunk/server/util_pcre.c
>
> Modified: httpd/httpd/trunk/server/util_pcre.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1612653&r1=1612652&r2=1612653&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_pcre.c (original)
> +++ httpd/httpd/trunk/server/util_pcre.c Tue Jul 22 19:29:08 2014
> @@ -125,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t *
> const char *errorptr;
> int erroffset;
> int errcode = 0;
> - /* PCRE_DUPNAMES is only present in more recent versions of PCRE */
> + /* PCRE_DUPNAMES is only present since version 6.7 of PCRE */
> #ifdef PCRE_DUPNAMES
> int options = PCRE_DUPNAMES;
> #else
>
>
>