Return-Path: Delivered-To: apmail-httpd-dev-archive@www.apache.org Received: (qmail 89274 invoked from network); 21 Sep 2004 14:37:03 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur-2.apache.org with SMTP; 21 Sep 2004 14:37:03 -0000 Received: (qmail 6085 invoked by uid 500); 21 Sep 2004 14:35:16 -0000 Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 5882 invoked by uid 500); 21 Sep 2004 14:35:15 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: list-post: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 5820 invoked by uid 99); 21 Sep 2004 14:35:14 -0000 X-ASF-Spam-Status: No, hits=0.3 required=10.0 tests=DNS_FROM_RFC_ABUSE,HTML_40_50,HTML_MESSAGE,HTML_TITLE_EMPTY X-Spam-Check-By: apache.org Received-SPF: pass (hermes.apache.org: local policy) Received: from [12.11.148.32] (HELO exrelay.ptc.com) (12.11.148.32) by apache.org (qpsmtpd/0.28) with ESMTP; Tue, 21 Sep 2004 07:35:10 -0700 Received: from hq-exfe1.ptcnet.ptc.com (132.253.201.60) by exrelay.ptc.com with ESMTP; 21 Sep 2004 10:35:11 -0400 X-Ironport-AV: i="3.84,170,1091419200"; d="scan'217,208"; a="796914:sNHT20309864" Received: from [132.253.10.127] ([132.253.10.127]) by HQ-EXFE1.ptcnet.ptc.com with Microsoft SMTPSVC(5.0.2195.6713); Tue, 21 Sep 2004 10:35:03 -0400 Message-ID: <41503C16.7080907@ptc.com> Date: Tue, 21 Sep 2004 09:35:02 -0500 From: Jess Holle User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910 X-Accept-Language: en-us, en MIME-Version: 1.0 To: dev@httpd.apache.org Subject: Re: cvs commit: httpd-2.0/modules/ldap util_ldap_cache_mgr.c References: In-Reply-To: Content-Type: multipart/alternative; boundary="------------020806060203070001040409" X-OriginalArrivalTime: 21 Sep 2004 14:35:03.0368 (UTC) FILETIME=[2E722C80:01C49FE8] X-Virus-Checked: Checked X-Spam-Rating: minotaur-2.apache.org 1.6.2 0/1000/N This is a multi-part message in MIME format. --------------020806060203070001040409 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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 > > > >>>>jessh@ptc.com 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 -0000, minfrin@apache.org wrote: >> >> >> >> >>> --- util_ldap_cache_mgr.c 13 Sep 2004 11:11:32 -0000 1.8 >>> +++ util_ldap_cache_mgr.c 19 Sep 2004 23:00:25 -0000 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? >> >> >> >> > > > --------------020806060203070001040409 Content-Type: text/html; charset=us-ascii Content-Transfer-Encoding: 7bit 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 

  
jessh@ptc.com 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 -0000, minfrin@apache.org wrote:
 

    
 --- util_ldap_cache_mgr.c	13 Sep 2004 11:11:32 -0000	1.8
 +++ util_ldap_cache_mgr.c	19 Sep 2004 23:00:25 -0000	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?

--------------020806060203070001040409--