On Tue, Mar 18, 2008 at 6:01 PM, Nick Kew <[EMAIL PROTECTED]> wrote: > On Fri, 07 Mar 2008 21:02:42 -0000 > [EMAIL PROTECTED] wrote: > > > > Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c > > URL: > > > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=634821&r1=634820&r2=634821&view=diff > > > ============================================================================== > > --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original) +++ > > httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Mar 7 13:02:41 2008 > > @@ -1462,13 +1462,10 @@ /* ...and entry is valid */ > > *binddn = apr_pstrdup(r->pool, search_nodep->dn); > > if (attrs) { > > - int i = 0, k = 0; > > - while (attrs[k++]); > > - *retvals = apr_pcalloc(r->pool, sizeof(char *) * > > k); > > - while (search_nodep->vals[i]) { > > - (*retvals)[i] = apr_pstrdup(r->pool, > > - > > search_nodep->vals[i]); > > - i++; > > + int i; > > + *retvals = apr_pcalloc(r->pool, sizeof(char *) * > > search_nodep->numvals); > > + for (i = 0; i < search_nodep->numvals; i++) { > > + (*retvals)[i] = apr_pstrdup(r->pool, > > search_nodep->vals[i]); } > > Um, doesn't that need a not-null test to avoid a segfault? > If the switch from while(...) to for(...) does anything, > it must be 'cos there's a null in there, yesno?
Yes, vals[] can be swiss-cheese and the old loop would stop processing early (attrs[], not vals[], is null terminated per the ldap API) apr_pstrdup() accomodates for the null parameter down to .9.x > > Also, because I CBA to figure it out, I take it the > search_nodep->vals[i] must live on the stack and/or get > overwritten, so the copying is necessary? The copying is necessary because these strings live in the shared memory util_ldap/mod_ldap cache. The caller (authnz_ldap) doesn't directly hold a cache lock, so there is no chance later on to copy this info out safely. -- Eric Covener [EMAIL PROTECTED]
