Re: cvs commit: httpd-2.0/modules/ldap util_ldap_cache_mgr.c
Ah Yes, it would be good to have the same fix in both branches. Sorry -- I overlooked this... -- Jess Holle Brad Nicholes wrote: Even though it isn't a big deal, the point is that newcurl is not undefined. It was initialized to NULL when it was declared at the beginning of the function. This is a mismatch between HEAD and APACHE_2_0_BRANCH. util_ldap has been moved out of experimental in HEAD and now exists in modules/ldap. jorton is refering to the r1.7 revision of modules/ldap/util_ldap_cache_mgr.c rather than modules/experimental/util_ldap_cache_mgr.c. The backport should include the initialization of newcurl as was committed in modules/ldap/util_ldap_cache_mgr.c r1.7 rather than the "else" code submitted in the patch file so that the backport matches the current state of the code. Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com [EMAIL PROTECTED] Monday, September 20, 2004 7:15:27 AM See my util_ldap.c patch. Whether or not this patch is absolutely necessary, leaving code paths that leave function return results undefined is never good! Also, see the bigger patch to util_ldap_cache_mgr.c that I passed along yesterday. I've attached both here for convenience. -- Jess Holle Joe Orton wrote: On Sun, Sep 19, 2004 at 11:00:25PM -, [EMAIL PROTECTED] wrote: --- util_ldap_cache_mgr.c 13 Sep 2004 11:11:32 - 1.8 +++ util_ldap_cache_mgr.c 19 Sep 2004 23:00:25 - 1.9 ... @@ -252,6 +254,8 @@ newcurl = util_ald_cache_insert(st-util_ldap_cache, curl); } +else + newcurl = NULL; return newcurl; } This is not necessary after the r1.7 fix last week, so there's no point in asking for r1.7 to be back-ported as well as this. Also it looks as if the _child_init hook in util_ldap.c will still segfault with a NULL _cache_lock?
Re: cvs commit: httpd-2.0/modules/ldap util_ldap_cache_mgr.c
On Sun, Sep 19, 2004 at 11:00:25PM -, [EMAIL PROTECTED] wrote: --- util_ldap_cache_mgr.c 13 Sep 2004 11:11:32 - 1.8 +++ util_ldap_cache_mgr.c 19 Sep 2004 23:00:25 - 1.9 ... @@ -252,6 +254,8 @@ newcurl = util_ald_cache_insert(st-util_ldap_cache, curl); } +else + newcurl = NULL; return newcurl; } This is not necessary after the r1.7 fix last week, so there's no point in asking for r1.7 to be back-ported as well as this. Also it looks as if the _child_init hook in util_ldap.c will still segfault with a NULL _cache_lock?
Re: cvs commit: httpd-2.0/modules/ldap util_ldap_cache_mgr.c
See my util_ldap.c patch.
Whether or not this patch is absolutely necessary, leaving code paths
that leave function return results undefined is never good!
Also, see the bigger patch to util_ldap_cache_mgr.c that I passed along
yesterday.
I've attached both here for convenience.
--
Jess Holle
Joe Orton wrote:
On Sun, Sep 19, 2004 at 11:00:25PM -, [EMAIL PROTECTED] wrote:
--- util_ldap_cache_mgr.c 13 Sep 2004 11:11:32 - 1.8
+++ util_ldap_cache_mgr.c 19 Sep 2004 23:00:25 - 1.9
...
@@ -252,6 +254,8 @@
newcurl = util_ald_cache_insert(st-util_ldap_cache, curl);
}
+else
+ newcurl = NULL;
return newcurl;
}
This is not necessary after the r1.7 fix last week, so there's no point
in asking for r1.7 to be back-ported as well as this.
Also it looks as if the _child_init hook in util_ldap.c will still
segfault with a NULL _cache_lock?
--- util_ldap.c-2.0.51 2004-09-19 17:11:02.0 -0500
+++ util_ldap.c-new 2004-09-19 17:11:06.0 -0500
@@ -89,9 +89,11 @@
#endif
#define LDAP_CACHE_LOCK() \
-apr_global_mutex_lock(st-util_ldap_cache_lock)
+if (st-util_ldap_cache_lock) \
+ apr_global_mutex_lock(st-util_ldap_cache_lock)
#define LDAP_CACHE_UNLOCK() \
-apr_global_mutex_unlock(st-util_ldap_cache_lock)
+if (st-util_ldap_cache_lock) \
+ apr_global_mutex_unlock(st-util_ldap_cache_lock)
static void util_ldap_strdup (char **str, const char *newstr)
--- util_ldap_cache_mgr.c-2.0.512004-09-19 16:28:00.0 -0500
+++ util_ldap_cache_mgr.c 2004-09-19 16:27:56.0 -0500
@@ -173,7 +173,7 @@
void util_ald_cache_purge(util_ald_cache_t *cache)
{
unsigned long i;
-util_cache_node_t *p, *q;
+util_cache_node_t *p, *q, **pp;
apr_time_t t;
if (!cache)
@@ -184,7 +184,8 @@
cache-numpurges++;
for (i=0; i cache-size; ++i) {
-p = cache-nodes[i];
+pp = cache-nodes + i;
+p = *pp;
while (p != NULL) {
if (p-add_time cache-marktime) {
q = p-next;
@@ -192,10 +193,11 @@
util_ald_free(cache, p);
cache-numentries--;
cache-npurged++;
-p = q;
+p = *pp = q;
}
else {
-p = p-next;
+pp = (p-next);
+p = *pp;
}
}
}
@@ -252,6 +254,8 @@
newcurl = util_ald_cache_insert(st-util_ldap_cache, curl);
}
+else
+ newcurl = NULL;
return newcurl;
}
Re: cvs commit: httpd-2.0/modules/ldap util_ldap_cache_mgr.c
Even though it isn't a big deal, the point is that newcurl is not undefined. It was initialized to NULL when it was declared at the beginning of the function. This is a mismatch between HEAD and APACHE_2_0_BRANCH. util_ldap has been moved out of experimental in HEAD and now exists in modules/ldap. jorton is refering to the r1.7 revision of modules/ldap/util_ldap_cache_mgr.c rather than modules/experimental/util_ldap_cache_mgr.c. The backport should include the initialization of newcurl as was committed in modules/ldap/util_ldap_cache_mgr.c r1.7 rather than the else code submitted in the patch file so that the backport matches the current state of the code. Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com [EMAIL PROTECTED] Monday, September 20, 2004 7:15:27 AM See my util_ldap.c patch. Whether or not this patch is absolutely necessary, leaving code paths that leave function return results undefined is never good! Also, see the bigger patch to util_ldap_cache_mgr.c that I passed along yesterday. I've attached both here for convenience. -- Jess Holle Joe Orton wrote: On Sun, Sep 19, 2004 at 11:00:25PM -, [EMAIL PROTECTED] wrote: --- util_ldap_cache_mgr.c 13 Sep 2004 11:11:32 - 1.8 +++ util_ldap_cache_mgr.c 19 Sep 2004 23:00:25 - 1.9 ... @@ -252,6 +254,8 @@ newcurl = util_ald_cache_insert(st-util_ldap_cache, curl); } +else + newcurl = NULL; return newcurl; } This is not necessary after the r1.7 fix last week, so there's no point in asking for r1.7 to be back-ported as well as this. Also it looks as if the _child_init hook in util_ldap.c will still segfault with a NULL _cache_lock?
