httpd-apreq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Randy Kobes <ra...@theoryx5.uwinnipeg.ca>
Subject Re: [multi-env] t/parsers.c test on Win32
Date Wed, 09 Feb 2005 05:48:30 GMT
On Tue, 8 Feb 2005, Joe Schaefer wrote:

> Randy Kobes <randy@theoryx5.uwinnipeg.ca> writes:
>
> [...]
>
> > I may be missing something, but does that mean that
> > apreq_register_parser(NULL, NULL) effectively returns
> > without doing anything?
>
> You are correct, I think.  It should be safe to remove
> the "apreq_register_parser(NULL, NULL)" call.
>
> > I tried the following for t/parsers.c:
> > ======================================================
> > Index: t/parsers.c
> > ===================================================================
> > --- t/parsers.c	(revision 152645)
> > +++ t/parsers.c	(working copy)
> > @@ -109,6 +109,10 @@
> >
> >      /* initialize default-parser hash */
> >      apreq_register_parser(NULL, NULL);
> > +    apreq_register_parser(URL_ENCTYPE, apreq_parse_urlencoded);
> > +    apreq_register_parser(MFD_ENCTYPE, apreq_parse_multipart);
> > +    apreq_register_parser(MR_ENCTYPE, apreq_parse_multipart);
> > +
> >
> >      f = apreq_parser(URL_ENCTYPE);
> >      CuAssertPtrNotNull(tc, f);
> > ================================================================
> > and got all the tests to pass. However, I don't understand
> > why - I thought this was done in apreq_initialize(),
> > which is called by testall.c (and the status checked for
> > success).
>
> But we need to check the return values of the apreq_register_parser
> calls, especially in apreq_initialize, because apr threads may
> not be reliable on Win32 (what's your apr version btw)?

It's 0.9.5, from Apache/2.0.52.

> I'd start by patching apreq_initialize and seeing what
> happens from there.  If we can't identify the bug,
> lets consider either removing the new thread-locking
> stuff or determining which libapr version has reliable
> behavior.

I tried putting in some additional checks at various
stages in src/apreq_parsers.c:

=======================================================
Index: apreq_parsers.c
===================================================================
--- apreq_parsers.c	(revision 152979)
+++ apreq_parsers.c	(working copy)
@@ -129,12 +129,21 @@
         return status;

     default_parsers = apr_hash_make(default_parser_pool);
+    if (default_parsers == NULL)
+        return APR_EGENERAL;

-    apreq_register_parser("application/x-www-form-urlencoded",
-                          apreq_parse_urlencoded);
-    apreq_register_parser("multipart/form-data", apreq_parse_multipart);
-    apreq_register_parser("multipart/related", apreq_parse_multipart);
-
+    status = apreq_register_parser("application/x-www-form-urlencoded",
+                                   apreq_parse_urlencoded);
+    if (status != APR_SUCCESS)
+        return status;
+    status = apreq_register_parser("multipart/form-data",
+                                   apreq_parse_multipart);
+    if (status != APR_SUCCESS)
+        return status;
+    status = apreq_register_parser("multipart/related",
+                                   apreq_parse_multipart);
+    if (status != APR_SUCCESS)
+        return status;
     return APR_SUCCESS;
 }

@@ -165,7 +174,9 @@
     apr_hash_set(default_parsers, apr_pstrdup(default_parser_pool, enctype),
                  APR_HASH_KEY_STRING, f);

-    apr_thread_rwlock_unlock(default_parsers_lock);
+    status = apr_thread_rwlock_unlock(default_parsers_lock);
+    if (status != APR_SUCCESS)
+        return status;

     return APR_SUCCESS;
 }
@@ -187,7 +198,9 @@

     f = apr_hash_get(default_parsers, enctype, tlen);

-    apr_thread_rwlock_unlock(default_parsers_lock);
+    status = apr_thread_rwlock_unlock(default_parsers_lock);
+    if (status != APR_SUCCESS)
+        return NULL;

     if (f != NULL)
         return *f;

================================================================
and then called apreq_initialize() from t/parsers.c (and
subsequently taking it out from testall.c):
===============================================================
Index: ../t/parsers.c
===================================================================
--- ../t/parsers.c	(revision 152979)
+++ ../t/parsers.c	(working copy)
@@ -106,9 +106,9 @@
 static void locate_default_parsers(CuTest *tc)
 {
     apreq_parser_function_t f;
-
-    /* initialize default-parser hash */
-    apreq_register_parser(NULL, NULL);
+    apr_status_t status;
+    status = apreq_initialize(p);
+    CuAssertIntEquals(tc, status, APR_SUCCESS);

     f = apreq_parser(URL_ENCTYPE);
     CuAssertPtrNotNull(tc, f);

=======================================================
It still fails in the Parsers test:
1) locate_default_parsers: expected pointer <0040372A>
   but was <10004280>

However, with these checks, apreq_initialize() was
APR_SUCCESS, and f != NULL.

-- 
best regards,
randy

Mime
View raw message