You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2007/11/17 10:04:54 UTC

Re: svn commit: r595866 - /httpd/httpd/trunk/modules/ldap/util_ldap_cache.c


On 11/17/2007 12:15 AM, rederpj@apache.org wrote:
> Author: rederpj
> Date: Fri Nov 16 15:14:56 2007
> New Revision: 595866
> 
> URL: http://svn.apache.org/viewvc?rev=595866&view=rev
> Log:
> A quick fix to avoid potential memory issues.
> 
> Modified:
>     httpd/httpd/trunk/modules/ldap/util_ldap_cache.c
> 
> Modified: httpd/httpd/trunk/modules/ldap/util_ldap_cache.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap_cache.c?rev=595866&r1=595865&r2=595866&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ldap/util_ldap_cache.c (original)
> +++ httpd/httpd/trunk/modules/ldap/util_ldap_cache.c Fri Nov 16 15:14:56 2007
> @@ -266,7 +266,13 @@
>          }
>          node->lastcompare = n->lastcompare;
>          node->result = n->result;
> -        node->sgl_processed = n->sgl_processed;
> +        if (n->subgroupList && !node->subgroupList ) {

Sorry, but I do not understand how this could happen, as a few lines above we
have:
        if (!(node->dn = util_ald_strdup(cache, n->dn)) ||
            !(node->attrib = util_ald_strdup(cache, n->attrib)) ||
            !(node->value = util_ald_strdup(cache, n->value)) ||
            ((n->subgroupList) && !(node->subgroupList = util_ald_sgl_dup(cache, n->subgroupList)))) {
               util_ldap_compare_node_free(cache, node);
               return NULL;
        }

So IMHO the condition in the new if condition never becomes true and thus the new code
would be the same as the old one.

Regards

RĂ¼diger


Re: svn commit: r595866 - /httpd/httpd/trunk/modules/ldap/util_ldap_cache.c

Posted by "Paul J. Reder" <re...@remulak.net>.
Long story. Good catch. It's coming back out.

Ruediger Pluem wrote:
> 
> On 11/17/2007 12:15 AM, rederpj@apache.org wrote:
>> Author: rederpj
>> Date: Fri Nov 16 15:14:56 2007
>> New Revision: 595866
>>
>> URL: http://svn.apache.org/viewvc?rev=595866&view=rev
>> Log:
>> A quick fix to avoid potential memory issues.
>>
>> Modified:
>>     httpd/httpd/trunk/modules/ldap/util_ldap_cache.c
>>
>> Modified: httpd/httpd/trunk/modules/ldap/util_ldap_cache.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap_cache.c?rev=595866&r1=595865&r2=595866&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/ldap/util_ldap_cache.c (original)
>> +++ httpd/httpd/trunk/modules/ldap/util_ldap_cache.c Fri Nov 16 15:14:56 2007
>> @@ -266,7 +266,13 @@
>>          }
>>          node->lastcompare = n->lastcompare;
>>          node->result = n->result;
>> -        node->sgl_processed = n->sgl_processed;
>> +        if (n->subgroupList && !node->subgroupList ) {
> 
> Sorry, but I do not understand how this could happen, as a few lines above we
> have:
>         if (!(node->dn = util_ald_strdup(cache, n->dn)) ||
>             !(node->attrib = util_ald_strdup(cache, n->attrib)) ||
>             !(node->value = util_ald_strdup(cache, n->value)) ||
>             ((n->subgroupList) && !(node->subgroupList = util_ald_sgl_dup(cache, n->subgroupList)))) {
>                util_ldap_compare_node_free(cache, node);
>                return NULL;
>         }
> 
> So IMHO the condition in the new if condition never becomes true and thus the new code
> would be the same as the old one.
> 
> Regards
> 
> RĂ¼diger
> 
> 

-- 
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it.  Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein