Re: Making test framework and trunk approachable (was: PCRE 10 and puzzling edge cases)

2016-12-15 Thread Petr Pisar
On Mon, Dec 12, 2016 at 01:03:23PM -0600, William A Rowe Jr wrote:
> System APR does require the -devel package. That RHEL package aught to
> install the .m4 logic required by httpd, but it is on us to find that in a
> conventional place.
> 
I think this should be handled in the buildconf script. The apr-devel package
puts find_apr.m4 into /usr/share/aclocal (check "aclocal --print-ac-dir"
output).

> Httpd trunk is already hunting for yet unreleased APR 1.6 features, but we
> should better document that.
> 
That can happen. Then there is nothing to improve.

> Not yet installed httpd is a little more complex, maybe we can add an
> interim apxs in the build tree to facilitate this?
>
Something like libtool allows to run executables linked to just-built
libraries. Maybe.

But I think the hardest issue is the modules sources are scattered across many
directories and the just-built module DSOs too. Tests should have to be
smarter in locating them.

-- Petr


signature.asc
Description: PGP signature


Re: PCRE 10 and puzzling edge cases

2016-12-12 Thread Petr Pisar
On Fri, Dec 09, 2016 at 01:07:54PM -0600, William A Rowe Jr wrote:
> 
> Thanks again, if there is anything that we didn't document well about
> getting the tests to run, we would like to fix that and make it easier for
> developers to cobble together their own test environment. We certainly don't
> want it to take hours for an experienced newcomer to get from point a to
> point b.
> 
First problem was how to generate ./configure from SVN sources. Obvious
autoreconf or autoconf did not work because of some missing macros. Then
I found ./buildconf.

But it still failed because of missing APR sources despite I had installed
APR 1.5.4 including header files and M4 macros from my distribution packages.
Then I found in the documentation that building from SVN requires APR sources.
So I cloned APR sources. Then I was able to build httpd.

I wanted to run test, but there are no tests you complained about a failure.
After searching web, I got an idea that httpd-framework is what I need.

I made sure I have installed all Perl modules I found relevant, but I was
unable to run the tests against SVN httpd sources. I played with
LD_LIBRARY_PATH, apxs etc. but without any good result. At the end
I reconfigured httpd sources and installed the binaries into a /tmp
subdirectory and that allowed me to run the tests. It's quite annoying to have
to install httpd into the system just only to test it.

But still the tests failed because of an undefined symbol in a session test
module. The symbol was defined in the httpd session module but the session
module was not named by the LoadModule keyword in the generated
t/conf/httpd.conf. I did not found a way how to disable the session tests
other than removing c-modules/test_session/mod_test_session.c file.

Finally I obtained a pass.

I think the inability to build httpd against system APR and to run test
against not yet installed httpd is quite surprising.

-- Petr


signature.asc
Description: PGP signature


Re: PCRE 10 and puzzling edge cases

2016-12-09 Thread Petr Pisar
On Thu, Dec 08, 2016 at 11:09:42AM -0600, William A Rowe Jr wrote:
> I've beaten my head against this wall for a bit longer, and came up with
> several places where pcre2 changed return types for void *what query
> interogations (especially from int to uint32, badness on x86_64-linux).
> 
> The attached patch picks up these bad void * type assignments. Still
> no tremendous improvement, missing something blatantly obvious,
> I expect.
> 
After few hours of getting httpd sources and tests and hacking them to
actually obtain a pass, I applied your patch and looked what's wrong with your
PCRE2 code.

The t/apache/expr.t failed because the pcre2_match() alwayes returned -51
that means PCRE2_ERROR_NULL. The reason is you used the old PCRE optimization
path and called pcre2_match_data_create(nmatch, NULL) only if nmatch was
positive. As a result, pcre2_match_data_create() was never called, so you
passed NULL matchdata to pcre2_match(), hence the failure.

See the attached fix.

The tests still do not pass, but that's probably another (PCRE2) problem.
I hope I helped you at lest somehow.

-- Petr
From 5e28aa58b19fdbfe485f50668c3176fe23e25609 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= 
Date: Fri, 9 Dec 2016 14:59:57 +0100
Subject: [PATCH] Fix never match
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PCRE2 always requires pcre2_match_data_create(). Otherwise
pcre2_match() returns PCRE2_ERROR_NULL.

Signed-off-by: Petr Písař 
---
 server/util_pcre.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/server/util_pcre.c b/server/util_pcre.c
index 845dd33..0c435ff 100644
--- a/server/util_pcre.c
+++ b/server/util_pcre.c
@@ -248,13 +248,13 @@ AP_DECLARE(int) ap_regexec_len(const ap_regex_t *preg, 
const char *buff,
 
 ((ap_regex_t *)preg)->re_erroffset = (apr_size_t)(-1);/* Only has 
meaning after compile */
 
-if (nmatch > 0) {
 #ifdef HAVE_PCRE2
-matchdata = pcre2_match_data_create(nmatch, NULL);
-if (matchdata == NULL)
-return AP_REG_ESPACE;
-ovector = pcre2_get_ovector_pointer(matchdata);
+matchdata = pcre2_match_data_create(nmatch, NULL);
+if (matchdata == NULL)
+return AP_REG_ESPACE;
+ovector = pcre2_get_ovector_pointer(matchdata);
 #else
+if (nmatch > 0) {
 if (nmatch <= POSIX_MALLOC_THRESHOLD) {
 ovector = &(small_ovector[0]);
 }
@@ -264,8 +264,8 @@ AP_DECLARE(int) ap_regexec_len(const ap_regex_t *preg, 
const char *buff,
 return AP_REG_ESPACE;
 allocated_ovector = 1;
 }
-#endif
 }
+#endif
 
 #ifdef HAVE_PCRE2
 rc = pcre2_match((const pcre2_code *)preg->re_pcre,
@@ -290,8 +290,7 @@ AP_DECLARE(int) ap_regexec_len(const ap_regex_t *preg, 
const char *buff,
 }
 
 #ifdef HAVE_PCRE2
-if (matchdata)
-pcre2_match_data_free(matchdata);
+pcre2_match_data_free(matchdata);
 #else
 if (allocated_ovector)
 free(ovector);
-- 
2.7.4



signature.asc
Description: PGP signature