[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: Possible patch to slapd/back-ldbm/cache.c



According to Daniel Carroll:
> 
> 
> Here's the patch to the cache_delete_entry_internal() routine
> that I mentioned in my other message:
> 
Maybe I'm just missing the wood for the trees, but the only change I 
detect in your patch is the deletion of the error initialisation (I'm 
not checking this against the code, just going on your posting), and 
the deletion of a peculiar return test.  Off the cuff, deleting the 
initialisation may be right, but given that this also implies turning 
"error" from a local to a global variable, I think the patch should be 
inspected carefully.  The deletion of the odd return code makes me 
think that the original thinking was a bit twisted.

The other changes are invisible.  There's an option to diff/cdiff that 
ignores whitespace differences, in this case, you should probably use 
it to make your patch more compact and clearer.

That said, I'd better re-read your previous message to find out what it 
is that you're correcting, and go have a look at the code you're 
modifying.

Please don't take the above as criticism, I'm just checking your 
arithmetic :-)

(patch intentionally included for reference).
> 	- Dan
> 
> *** ldap/servers/slapd/back-ldbm/cache.c	Thu Dec 17 13:16:40 1998
> --- ldap/servers/slapd/back-ldbm/cache.c.orig	Thu Dec 17 13:12:41 1998
> ***************
> *** 298,317 ****
>       Entry		*e
>   )
>   {
> - 	int error = 0;
> - 
>   	/* dn tree */
>   	if ( avl_delete( &cache->c_dntree, e, cache_entrydn_cmp ) == NULL ) {
> ! 		error = -1;
>   	}
>   
>   	/* id tree */
>   	if ( avl_delete( &cache->c_idtree, e, cache_entryid_cmp ) == NULL ) {
> ! 		error = -1;
>   	}
> - 
> - 	if (error == -1)
> - 		return ( -1 );
>   
>   	/* lru */
>   	LRU_DELETE( cache, e );
> --- 298,312 ----
>       Entry		*e
>   )
>   {
>   	/* dn tree */
>   	if ( avl_delete( &cache->c_dntree, e, cache_entrydn_cmp ) == NULL ) {
> ! 		return( -1 );
>   	}
>   
>   	/* id tree */
>   	if ( avl_delete( &cache->c_idtree, e, cache_entryid_cmp ) == NULL ) {
> ! 		return( -1 );
>   	}
>   
>   	/* lru */
>   	LRU_DELETE( cache, e );
>