Fixed a memory leak and getting ready to reuse some code between

modify and modrdn.

servers/slapd/back-bdb2/modify.c

@@ -12,32 +12,101 @@
 #include "proto-back-bdb2.h"
 
 
+static void	add_lastmods(Operation *op, LDAPModList **ml);
+
+
+static void
+add_lastmods( Operation *op, LDAPModList **modlist )
+{
+	char		buf[22];
+	struct berval	bv;
+	struct berval	*bvals[2];
+	LDAPModList		**m;
+	LDAPModList		*tmp;
+	struct tm	*ltm;
+	time_t		currenttime;
+
+	Debug( LDAP_DEBUG_TRACE, "add_lastmods\n", 0, 0, 0 );
+
+	bvals[0] = &bv;
+	bvals[1] = NULL;
+
+	/* remove any attempts by the user to modify these attrs */
+	for ( m = modlist; *m != NULL; m = &(*m)->ml_next ) {
+            if ( strcasecmp( (*m)->ml_type, "modifytimestamp" ) == 0 || 
+				strcasecmp( (*m)->ml_type, "modifiersname" ) == 0 ||
+				strcasecmp( (*m)->ml_type, "createtimestamp" ) == 0 || 
+				strcasecmp( (*m)->ml_type, "creatorsname" ) == 0 ) {
+
+                Debug( LDAP_DEBUG_TRACE,
+					"add_lastmods: found lastmod attr: %s\n",
+					(*m)->ml_type, 0, 0 );
+                tmp = *m;
+                *m = (*m)->ml_next;
+                free( tmp->ml_type );
+                if ( tmp->ml_bvalues != NULL ) {
+                    ber_bvecfree( tmp->ml_bvalues );
+                }
+                free( tmp );
+                if (!*m)
+                    break;
+            }
+        }
+
+	if ( op->o_dn == NULL || op->o_dn[0] == '\0' ) {
+		bv.bv_val = "NULLDN";
+		bv.bv_len = strlen( bv.bv_val );
+	} else {
+		bv.bv_val = op->o_dn;
+		bv.bv_len = strlen( bv.bv_val );
+	}
+	tmp = (LDAPModList *) ch_calloc( 1, sizeof(LDAPModList) );
+	tmp->ml_type = ch_strdup( "modifiersname" );
+	tmp->ml_op = LDAP_MOD_REPLACE;
+	tmp->ml_bvalues = (struct berval **) ch_calloc(2, sizeof(struct berval *));
+	tmp->ml_bvalues[0] = ber_bvdup( &bv );
+	tmp->ml_next = *modlist;
+	*modlist = tmp;
+
+	currenttime = slap_get_time();
+	ldap_pvt_thread_mutex_lock( &gmtime_mutex );
+#ifndef LDAP_LOCALTIME
+	ltm = gmtime( &currenttime );
+	strftime( buf, sizeof(buf), "%Y%m%d%H%M%SZ", ltm );
+#else
+	ltm = localtime( &currenttime );
+	strftime( buf, sizeof(buf), "%y%m%d%H%M%SZ", ltm );
+#endif
+	ldap_pvt_thread_mutex_unlock( &gmtime_mutex );
+
+	bv.bv_val = buf;
+	bv.bv_len = strlen( bv.bv_val );
+	tmp = (LDAPModList *) ch_calloc( 1, sizeof(LDAPModList) );
+	tmp->ml_type = ch_strdup( "modifytimestamp" );
+	tmp->ml_op = LDAP_MOD_REPLACE;
+	tmp->ml_bvalues = (struct berval **) ch_calloc(2, sizeof(struct berval *));
+	tmp->ml_bvalues[0] = ber_bvdup( &bv );
+	tmp->ml_next = *modlist;
+	*modlist = tmp;
+
+}
+
 static int
 bdb2i_back_modify_internal(
     BackendDB	*be,
     Connection	*conn,
     Operation	*op,
     char	*dn,
-    LDAPModList	*modlist
+    LDAPModList	*modlist,
+    Entry	 *e
 )
 {
 	struct ldbminfo	*li = (struct ldbminfo *) be->be_private;
-	char		*matched;
 	LDAPModList	*ml;
-	Entry		*e;
 	int		err;
 
 	Debug(LDAP_DEBUG_ARGS, "bdb2i_back_modify:\n", 0, 0, 0);
 
-	if ( (e = bdb2i_dn2entry_w( be, dn, &matched )) == NULL ) {
-		send_ldap_result( conn, op, LDAP_NO_SUCH_OBJECT, matched,
-		    NULL );
-		if ( matched != NULL ) {
-			free( matched );
-		}
-		return( -1 );
-	}
-
 	if ( (err = acl_check_modlist( be, conn, op, e, modlist )) != LDAP_SUCCESS ) {
 		send_ldap_result( conn, op, err, NULL, NULL );
 		goto error_return;
@@ -125,6 +194,7 @@ bdb2_back_modify(
 	struct ldbminfo	*li = (struct ldbminfo *) be->be_private;
 	struct timeval  time1;
 	int             ret;
+	char		*matched;
 
 	bdb2i_start_timing( be->bd_info, &time1 );
 
@@ -146,7 +216,16 @@ bdb2_back_modify(
 			break;
 	}
 
-	 ret = bdb2i_back_modify_internal( be, conn, op, dn, modlist );
+	if ( (e = bdb2i_dn2entry_w( be, dn, &matched )) == NULL ) {
+		send_ldap_result( conn, op, LDAP_NO_SUCH_OBJECT, matched,
+		    NULL );
+		if ( matched != NULL ) {
+			free( matched );
+		}
+		return( -1 );
+	}
+
+	 ret = bdb2i_back_modify_internal( be, conn, op, dn, modlist, e );
 	(void) bdb2i_leave_backend_w( lock );
 	bdb2i_stop_timing( be->bd_info, time1, "MOD", conn, op );
 

servers/slapd/back-bdb2/modrdn.c

@@ -201,12 +201,6 @@ bdb2i_back_modrdn_internal(
 	}
 	ldap_pvt_thread_mutex_unlock( &op->o_abandonmutex );
 
-	/* add new one */
-	if ( bdb2i_dn2id_add( be, new_ndn, e->e_id ) != 0 ) {
-		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, NULL, NULL );
-		goto return_results;
-	}
-
 	/* delete old one */
 	if ( bdb2i_dn2id_delete( be, e->e_ndn ) != 0 ) {
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, NULL, NULL );
@@ -219,6 +213,13 @@ bdb2i_back_modrdn_internal(
 	e->e_dn = new_dn;
 	e->e_ndn = new_ndn;
 
+
+	/* add new one */
+	if ( bdb2i_dn2id_add( be,  e->e_ndn, e->e_id ) != 0 ) {
+		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, NULL, NULL );
+		goto return_results;
+	}
+
 	/* Get attribute type and attribute value of our new rdn, we will
 	 * need to add that to our new entry
 	 */
@@ -376,17 +377,25 @@ bdb2i_back_modrdn_internal(
 
 	(void) bdb2i_cache_update_entry( &li->li_cache, e );
 
+	/* NOTE: after this you must not free new_dn or new_ndn!
+	 * They are used by cache.
+	 */
+
 	/* id2entry index */
 	if ( bdb2i_id2entry_add( be, e ) != 0 ) {
 		entry_free( e );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
-		goto return_results;
+		goto return_results_after;
 	}
 
 	send_ldap_result( conn, op, LDAP_SUCCESS, NULL, NULL );
 	rc = 0;
+	goto return_results_after;	
 
 return_results:
+	if( new_dn != NULL ) free( new_dn );
+	if( new_ndn != NULL ) free( new_ndn );
+return_results_after:
 	/* NOTE:
 	 * new_dn and new_ndn are not deallocated because they are used by
 	 * the cache entry.

servers/slapd/back-ldbm/modify.c

@@ -11,41 +11,125 @@
 #include "back-ldbm.h"
 #include "proto-back-ldbm.h"
 
+static void	add_lastmods(Operation *op, LDAPModList **ml);
 
-int
+
-ldbm_back_modify(
+static void
+add_lastmods( Operation *op, LDAPModList **modlist )
+{
+	char		buf[22];
+	struct berval	bv;
+	struct berval	*bvals[2];
+	LDAPModList		**m;
+	LDAPModList		*tmp;
+	struct tm	*ltm;
+	time_t		currenttime;
+
+	Debug( LDAP_DEBUG_TRACE, "add_lastmods\n", 0, 0, 0 );
+
+	bvals[0] = &bv;
+	bvals[1] = NULL;
+
+	/* remove any attempts by the user to modify these attrs */
+	for ( m = modlist; *m != NULL; m = &(*m)->ml_next ) {
+            if ( strcasecmp( (*m)->ml_type, "modifytimestamp" ) == 0 || 
+				strcasecmp( (*m)->ml_type, "modifiersname" ) == 0 ||
+				strcasecmp( (*m)->ml_type, "createtimestamp" ) == 0 || 
+				strcasecmp( (*m)->ml_type, "creatorsname" ) == 0 ) {
+
+                Debug( LDAP_DEBUG_TRACE,
+					"add_lastmods: found lastmod attr: %s\n",
+					(*m)->ml_type, 0, 0 );
+                tmp = *m;
+                *m = (*m)->ml_next;
+                free( tmp->ml_type );
+                if ( tmp->ml_bvalues != NULL ) {
+                    ber_bvecfree( tmp->ml_bvalues );
+                }
+                free( tmp );
+                if (!*m)
+                    break;
+            }
+        }
+
+	if ( op->o_dn == NULL || op->o_dn[0] == '\0' ) {
+		bv.bv_val = "NULLDN";
+		bv.bv_len = strlen( bv.bv_val );
+	} else {
+		bv.bv_val = op->o_dn;
+		bv.bv_len = strlen( bv.bv_val );
+	}
+	tmp = (LDAPModList *) ch_calloc( 1, sizeof(LDAPModList) );
+	tmp->ml_type = ch_strdup( "modifiersname" );
+	tmp->ml_op = LDAP_MOD_REPLACE;
+	tmp->ml_bvalues = (struct berval **) ch_calloc(2, sizeof(struct berval *));
+	tmp->ml_bvalues[0] = ber_bvdup( &bv );
+	tmp->ml_next = *modlist;
+	*modlist = tmp;
+
+	currenttime = slap_get_time();
+	ldap_pvt_thread_mutex_lock( &gmtime_mutex );
+#ifndef LDAP_LOCALTIME
+	ltm = gmtime( &currenttime );
+	strftime( buf, sizeof(buf), "%Y%m%d%H%M%SZ", ltm );
+#else
+	ltm = localtime( &currenttime );
+	strftime( buf, sizeof(buf), "%y%m%d%H%M%SZ", ltm );
+#endif
+	ldap_pvt_thread_mutex_unlock( &gmtime_mutex );
+
+	bv.bv_val = buf;
+	bv.bv_len = strlen( bv.bv_val );
+	tmp = (LDAPModList *) ch_calloc( 1, sizeof(LDAPModList) );
+	tmp->ml_type = ch_strdup( "modifytimestamp" );
+	tmp->ml_op = LDAP_MOD_REPLACE;
+	tmp->ml_bvalues = (struct berval **) ch_calloc(2, sizeof(struct berval *));
+	tmp->ml_bvalues[0] = ber_bvdup( &bv );
+	tmp->ml_next = *modlist;
+	*modlist = tmp;
+
+}
+
+/* We need this function because of LDAP modrdn. If we do not 
+ * add this there would be a bunch of code replication here 
+ * and there and of course the likelihood of bugs increases.
+ * Juan C. Gomez (gomez@engr.sgi.com) 05/18/99
+ */ 
+
+int ldbm_internal_modify(
     Backend	*be,
     Connection	*conn,
     Operation	*op,
     char	*dn,
-    LDAPModList	*modlist
+    LDAPModList	*modlist,
+    Entry	*e 
 )
 {
-	struct ldbminfo	*li = (struct ldbminfo *) be->be_private;
+	int		i, err;
-	char		*matched;
+	LDAPMod		*mod;
 	LDAPModList	*ml;
-	Entry		*e;
-	int		err;
 
-	Debug(LDAP_DEBUG_ARGS, "ldbm_back_modify:\n", 0, 0, 0);
+	if ( ((be->be_lastmod == ON)
+	      || ((be->be_lastmod == UNDEFINED)&&(global_lastmod == ON)))
+	     && (be->be_update_ndn == NULL)) {
+
+	        /* XXX: It may be wrong, it changes mod time even if 
+		 * mod fails!
+		 */
+		add_lastmods( op, &modlist );
 
-	/* acquire and lock entry */
-	if ( (e = dn2entry_w( be, dn, &matched )) == NULL ) {
-		send_ldap_result( conn, op, LDAP_NO_SUCH_OBJECT, matched,
-		    NULL );
-		if ( matched != NULL ) {
-			free( matched );
-		}
-		return( -1 );
 	}
 
-	if ( (err = acl_check_modlist( be, conn, op, e, modlist )) != LDAP_SUCCESS ) {
+
+	if ( (err = acl_check_modlist( be, conn, op, e, modlist ))
+	     != LDAP_SUCCESS ) {
 		send_ldap_result( conn, op, err, NULL, NULL );
-		goto error_return;
+		return -1;
 	}
 
 	for ( ml = modlist; ml != NULL; ml = ml->ml_next ) {
-		LDAPMod	*mod = &ml->ml_mod;
+
+		mod = &ml->ml_mod;
 
 		switch ( mod->mod_op & ~LDAP_MOD_BVALUES ) {
 		case LDAP_MOD_ADD:
@@ -64,7 +148,7 @@ ldbm_back_modify(
 		if ( err != LDAP_SUCCESS ) {
 			/* unlock entry, delete from cache */
 			send_ldap_result( conn, op, err, NULL, NULL );
-			goto error_return;
+			return -1;
 		}
 	}
 
@@ -72,31 +156,71 @@ ldbm_back_modify(
 	if ( global_schemacheck && oc_schema_check( e ) != 0 ) {
 		Debug( LDAP_DEBUG_ANY, "entry failed schema check\n", 0, 0, 0 );
 		send_ldap_result( conn, op, LDAP_OBJECT_CLASS_VIOLATION, NULL, NULL );
-		goto error_return;
+		return -1;
 	}
 
 	/* check for abandon */
 	ldap_pvt_thread_mutex_lock( &op->o_abandonmutex );
 	if ( op->o_abandon ) {
 		ldap_pvt_thread_mutex_unlock( &op->o_abandonmutex );
-		goto error_return;
+		return -1;
 	}
 	ldap_pvt_thread_mutex_unlock( &op->o_abandonmutex );
 
 	/* modify indexes */
 	if ( index_add_mods( be, modlist, e->e_id ) != 0 ) {
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, NULL, NULL );
-		goto error_return;
+		return -1;
 	}
 
 	/* check for abandon */
 	ldap_pvt_thread_mutex_lock( &op->o_abandonmutex );
 	if ( op->o_abandon ) {
 		ldap_pvt_thread_mutex_unlock( &op->o_abandonmutex );
-		goto error_return;
+		return -1;
 	}
 	ldap_pvt_thread_mutex_unlock( &op->o_abandonmutex );
 
+	return 0;
+
+}/* int ldbm_internal_modify() */
+
+
+int
+ldbm_back_modify(
+    Backend	*be,
+    Connection	*conn,
+    Operation	*op,
+    char	*dn,
+    LDAPModList	*modlist
+)
+{
+	struct ldbminfo	*li = (struct ldbminfo *) be->be_private;
+	char		*matched;
+	Entry		*e;
+	int		err;
+
+	Debug(LDAP_DEBUG_ARGS, "ldbm_back_modify:\n", 0, 0, 0);
+
+	/* acquire and lock entry */
+	if ( (e = dn2entry_w( be, dn, &matched )) == NULL ) {
+		send_ldap_result( conn, op, LDAP_NO_SUCH_OBJECT, matched,
+		    NULL );
+		if ( matched != NULL ) {
+			free( matched );
+		}
+		return( -1 );
+	}
+
+	/* Modify the entry */
+	if ( ldbm_internal_modify( be, conn, op, dn, modlist, e ) != 0 ) {
+
+		goto error_return;
+
+	}
+
+
+
 	/* change the entry itself */
 	if ( id2entry_add( be, e ) != 0 ) {
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, NULL, NULL );

servers/slapd/back-ldbm/modrdn.c

@@ -396,17 +396,25 @@ ldbm_back_modrdn(
 
 	(void) cache_update_entry( &li->li_cache, e );
 
+	/* NOTE: after this you must not free new_dn or new_ndn!
+	 * They are used by cache.
+	 */
+
 	/* id2entry index */
 	if ( id2entry_add( be, e ) != 0 ) {
 		entry_free( e );
 		send_ldap_result( conn, op, LDAP_OPERATIONS_ERROR, "", "" );
-		goto return_results;
+		goto return_results_after;
 	}
 
 	send_ldap_result( conn, op, LDAP_SUCCESS, NULL, NULL );
 	rc = 0;
+	goto return_results_after;	
 
 return_results:
+	if( new_dn != NULL ) free( new_dn );
+	if( new_ndn != NULL ) free( new_ndn );
+return_results_after:
 	/* NOTE:
 	 * new_dn and new_ndn are not deallocated because they are used by
 	 * the cache entry.

servers/slapd/back-ldbm/proto-back-ldbm.h

@@ -143,11 +143,19 @@ int index_add_values LDAP_P(( Backend *be, char *type, struct berval **vals, ID
  * modify.c
  * These prototypes are placed here because they are used by modify and
  * modify rdn which are implemented in different files. 
+ *
+ * We need ldbm_internal_modify here because of LDAP modrdn & modify use 
+ * it. If we do not add this, there would be a bunch of code replication 
+ * here and there and of course the likelihood of bugs increases.
+ * Juan C. Gomez (gomez@engr.sgi.com) 05/18/99
+ * 
  */
 
 int add_values LDAP_P(( Entry *e, LDAPMod *mod, char *dn ));
 int delete_values LDAP_P(( Entry *e, LDAPMod *mod, char *dn ));
 int replace_values LDAP_P(( Entry *e, LDAPMod *mod, char *dn ));
+int ldbm_internal_modify LDAP_P((Backend *be, Connection *conn, Operation *op,
+			         char *dn, LDAPModList *mods, Entry *e));
 
 /*
  * nextid.c

servers/slapd/modify.c

@@ -21,8 +21,6 @@
 #include "slap.h"
 
 static void	modlist_free(LDAPModList *ml);
-static void	add_lastmods(Operation *op, LDAPModList **ml);
-
 
 void
 do_modify(
@@ -157,10 +155,6 @@ do_modify(
 		if ( be->be_update_ndn == NULL ||
 			strcmp( be->be_update_ndn, op->o_ndn ) == 0 )
 		{
-			if ( (be->be_lastmod == ON || ( be->be_lastmod == UNDEFINED &&
-				global_lastmod == ON ) ) && be->be_update_ndn == NULL ) {
-				add_lastmods( op, &modlist );
-			}
 			if ( (*be->be_modify)( be, conn, op, ndn, modlist ) == 0 ) {
 				replog( be, LDAP_REQ_MODIFY, ndn, modlist, 0 );
 			}
@@ -196,78 +190,3 @@ modlist_free(
 		free( ml );
 	}
 }
-
-static void
-add_lastmods( Operation *op, LDAPModList **modlist )
-{
-	char		buf[22];
-	struct berval	bv;
-	struct berval	*bvals[2];
-	LDAPModList		**m;
-	LDAPModList		*tmp;
-	struct tm	*ltm;
-	time_t		currenttime;
-
-	Debug( LDAP_DEBUG_TRACE, "add_lastmods\n", 0, 0, 0 );
-
-	bvals[0] = &bv;
-	bvals[1] = NULL;
-
-	/* remove any attempts by the user to modify these attrs */
-	for ( m = modlist; *m != NULL; m = &(*m)->ml_next ) {
-            if ( strcasecmp( (*m)->ml_type, "modifytimestamp" ) == 0 || 
-				strcasecmp( (*m)->ml_type, "modifiersname" ) == 0 ||
-				strcasecmp( (*m)->ml_type, "createtimestamp" ) == 0 || 
-				strcasecmp( (*m)->ml_type, "creatorsname" ) == 0 ) {
-
-                Debug( LDAP_DEBUG_TRACE,
-					"add_lastmods: found lastmod attr: %s\n",
-					(*m)->ml_type, 0, 0 );
-                tmp = *m;
-                *m = (*m)->ml_next;
-                free( tmp->ml_type );
-                if ( tmp->ml_bvalues != NULL ) {
-                    ber_bvecfree( tmp->ml_bvalues );
-                }
-                free( tmp );
-                if (!*m)
-                    break;
-            }
-        }
-
-	if ( op->o_dn == NULL || op->o_dn[0] == '\0' ) {
-		bv.bv_val = "NULLDN";
-		bv.bv_len = strlen( bv.bv_val );
-	} else {
-		bv.bv_val = op->o_dn;
-		bv.bv_len = strlen( bv.bv_val );
-	}
-	tmp = (LDAPModList *) ch_calloc( 1, sizeof(LDAPModList) );
-	tmp->ml_type = ch_strdup( "modifiersname" );
-	tmp->ml_op = LDAP_MOD_REPLACE;
-	tmp->ml_bvalues = (struct berval **) ch_calloc(2, sizeof(struct berval *));
-	tmp->ml_bvalues[0] = ber_bvdup( &bv );
-	tmp->ml_next = *modlist;
-	*modlist = tmp;
-
-	currenttime = slap_get_time();
-	ldap_pvt_thread_mutex_lock( &gmtime_mutex );
-#ifndef LDAP_LOCALTIME
-	ltm = gmtime( &currenttime );
-	strftime( buf, sizeof(buf), "%Y%m%d%H%M%SZ", ltm );
-#else
-	ltm = localtime( &currenttime );
-	strftime( buf, sizeof(buf), "%y%m%d%H%M%SZ", ltm );
-#endif
-	ldap_pvt_thread_mutex_unlock( &gmtime_mutex );
-
-	bv.bv_val = buf;
-	bv.bv_len = strlen( bv.bv_val );
-	tmp = (LDAPModList *) ch_calloc( 1, sizeof(LDAPModList) );
-	tmp->ml_type = ch_strdup( "modifytimestamp" );
-	tmp->ml_op = LDAP_MOD_REPLACE;
-	tmp->ml_bvalues = (struct berval **) ch_calloc(2, sizeof(struct berval *));
-	tmp->ml_bvalues[0] = ber_bvdup( &bv );
-	tmp->ml_next = *modlist;
-	*modlist = tmp;
-}