httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Covener" <cove...@gmail.com>
Subject Re: svn commit: r634821 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c
Date Tue, 18 Mar 2008 22:24:04 GMT
On Tue, Mar 18, 2008 at 6:01 PM, Nick Kew <nick@webthing.com> wrote:
> On Fri, 07 Mar 2008 21:02:42 -0000
>  covener@apache.org 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
covener@gmail.com

Mime
View raw message