Alias/cache entry bugs (Was: Re: malloc bugs)

[Hallvard B Furuseth <h.b.furuseth@usit.uio.no>]

Will Ballantyne writes:
> I should have sent this to devel instead of replying...

I don't see why:-) but anyway, thanks for the fix - but now that I begin
to understand it, there is more:

This looks strange:
  for ( depth = 0;
	( (eMatched = dn2entry_r( be, newDN, &matched )) == NULL) ... ) {
    /* free reader lock */
    cache_return_entry_r(&li->li_cache, eMatched);

derefDN can hardly release a NULL entry to the cache:-) I expect it
should instead release it after the loop.  Also, the `eNew' entry should
be released at some time - I'm not quite sure when; I don't know how the
cache works.  Something like the untested patch below.

Also, derefAlias_r() returns either the Entry argument it receives, or
an entry fetched with dn2entry_r() - which I expect must be released to
the cache.  Which means that the caller must release the returned value
only if it is different from the argument it provided, which looks
confusing.  Maybe the function could return NULL instead if it would
return the argument.  Unless the NULL value must continue to mean error
("dangling alias"), as now.  (BTW, then I think it should return, not
continue to loop.)  Furthermore, if the function loops and does several
dn2entry_r()'s, it will not release the intermediate Entries to the
cache.

--- servers/slapd/back-ldbm/alias.c~	Wed Nov 25 07:52:59 1998
+++ servers/slapd/back-ldbm/alias.c	Wed Nov 25 08:47:16 1998
@@ -115,5 +115,5 @@
   char 	*matched;
   char 	*newDN = NULL;
-  int	depth;
+  int	depth, i;
   Entry 	*eMatched;
   Entry 	*eDeref;
@@ -133,7 +133,4 @@
 	++depth ) {
     
-    /* free reader lock */
-    cache_return_entry_r(&li->li_cache, eMatched);
-
     if ((matched != NULL) && *matched) {	
       char *submatch;
@@ -166,5 +163,8 @@
 	  Debug( LDAP_DEBUG_TRACE, "<= l&g we have %s vs %s \n", matched, eNew->e_dn, 0 );
 
-	  if (!strcasecmp (matched, eNew->e_dn)) {
+	  i = strcasecmp (matched, eNew->e_dn);
+          /* free reader lock */
+          cache_return_entry_r(&li->li_cache, eNew);
+	  if (! i) {
 	    /* newDN same as old so not an alias, no need to go further */
 	    free (newDN);
@@ -189,7 +189,4 @@
 	  matched = NULL;
 	  free (remainder);
-
-          /* free reader lock */
-          cache_return_entry_r(&li->li_cache, eNew);
 	}
         /* free reader lock */
@@ -206,4 +203,7 @@
   }
   
+  /* free reader lock */
+  cache_return_entry_r(&li->li_cache, eMatched);
+
   /*
    * the final part of the DN might be an alias