[SCM] Samba Shared Repository - branch master updated - tevent-0-9-8-931-g2ab27d7

Simo Sorce idra at samba.org
Fri Oct 2 15:41:05 MDT 2009


Mathias, why are yoiu adding all these talloc_free(ac); statements
everywhere ?

When you return an error from an operation normally the ldb_request is
freed, and the ac context is appended to the request so it gets also
automatically freed.

Are you actually seeing memory leaks ?
Or is there another reason to add these calls ?

Simo.

On Fri, 2009-10-02 at 16:31 -0500, Matthias Dieter Wallnöfer wrote:
> The branch, master has been updated
>        via  2ab27d78b0f2c10a6d632015090872a0f6542add (commit)
>       from  6f22cd10ad30bd9077916c4fecbc8f7bb08c68b9 (commit)
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -----------------------------------------------------------------
> commit 2ab27d78b0f2c10a6d632015090872a0f6542add
> Author: Matthias Dieter Wallnöfer <mwallnoefer at yahoo.de>
> Date:   Fri Oct 2 23:26:35 2009 +0200
> 
>     s4:repl_meta_data - various
>     
>     - Add more "talloc_free"s and right error values where needed
>     - Add a pre-lookup for entries before searching for metadata attribute
>       (also suggested by TODO list)
>     - Now the most part of "ldap.py" works again
> 
> -----------------------------------------------------------------------
> 
> Summary of changes:
>  source4/dsdb/samdb/ldb_modules/repl_meta_data.c |   47 ++++++++++++++++++----
>  1 files changed, 38 insertions(+), 9 deletions(-)
> 
> 
> Changeset truncated at 500 lines:
> 
> diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> index 59dc2ec..489985a 100644
> --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> @@ -514,12 +514,14 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  		if( !allow_add_guid ) {
>  			ldb_debug_set(ldb, LDB_DEBUG_ERROR,
> 			      "replmd_add: it's not allowed to add an object with objectGUID\n");
> +			talloc_free(ac);
>  			return LDB_ERR_UNWILLING_TO_PERFORM;
>  		} else {
>  			NTSTATUS status = GUID_from_data_blob(guid_blob,&guid);
>  		        if ( !NT_STATUS_IS_OK(status)) {
>         				ldb_debug_set(ldb, LDB_DEBUG_ERROR,
>  				      "replmd_add: Unable to parse as a GUID the attribute objectGUID\n");
> +				talloc_free(ac);
>  				return LDB_ERR_UNWILLING_TO_PERFORM;
>  			}
>  			/* we remove this attribute as it can be a string and will not be treated 
> @@ -534,15 +536,16 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	/* Get a sequence number from the backend */
>  	ret = ldb_sequence_number(ldb, LDB_SEQ_NEXT, &seq_num);
>  	if (ret != LDB_SUCCESS) {
> +		talloc_free(ac);
>  		return ret;
>  	}
>  
> -
>  	/* get our invocationId */
>  	our_invocation_id = samdb_ntds_invocation_id(ldb);
>  	if (!our_invocation_id) {
>  		ldb_debug_set(ldb, LDB_DEBUG_ERROR,
>  			      "replmd_add: unable to find invocationId\n");
> +		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
> @@ -550,6 +553,7 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	msg = ldb_msg_copy_shallow(ac, req->op.add.message);
>  	if (msg == NULL) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
> @@ -557,6 +561,8 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	unix_to_nt_time(&now, t);
>  	time_str = ldb_timestring(msg, t);
>  	if (!time_str) {
> +		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  	if (remove_current_guid) {
> @@ -576,7 +582,8 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  		ret = ldb_msg_add_fmt(msg, "instanceType", "%u", INSTANCE_TYPE_WRITE);
>  		if (ret != LDB_SUCCESS) {
>  			ldb_oom(ldb);
> -			return LDB_ERR_OPERATIONS_ERROR;
> +			talloc_free(ac);
> +			return ret;
>  		}
>  	}
>  
> @@ -586,7 +593,8 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	ret = ldb_msg_add_string(msg, "whenCreated", time_str);
>  	if (ret != LDB_SUCCESS) {
>  		ldb_oom(ldb);
> -		return LDB_ERR_OPERATIONS_ERROR;
> +		talloc_free(ac);
> +		return ret;
>  	}
>  
>  	/* build the replication meta_data */
> @@ -598,6 +606,7 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  					       nmd.ctr.ctr1.count);
>  	if (!nmd.ctr.ctr1.array) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
> @@ -613,6 +622,7 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  			ldb_debug_set(ldb, LDB_DEBUG_ERROR,
>  				      "replmd_add: attribute '%s' not defined in schema\n",
>  				      e->name);
> +			talloc_free(ac);
>  			return LDB_ERR_NO_SUCH_ATTRIBUTE;
>  		}
>  
> @@ -640,6 +650,7 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	 */
>  	ret = replmd_replPropertyMetaDataCtr1_sort(&nmd.ctr.ctr1, schema, msg->dn);
>  	if (ret != LDB_SUCCESS) {
> +		talloc_free(ac);
>  		return ret;
>  	}
>  
> @@ -650,6 +661,7 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  				       (ndr_push_flags_fn_t)ndr_push_GUID);
>  	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  	ndr_err = ndr_push_struct_blob(&nmd_value, msg, 
> @@ -658,6 +670,7 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  				       (ndr_push_flags_fn_t)ndr_push_replPropertyMetaDataBlob);
>  	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
> @@ -667,26 +680,31 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  	ret = ldb_msg_add_value(msg, "objectGUID", &guid_value, NULL);
>  	if (ret != LDB_SUCCESS) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return ret;
>  	}
>  	ret = ldb_msg_add_string(msg, "whenChanged", time_str);
>  	if (ret != LDB_SUCCESS) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return ret;
>  	}
>  	ret = samdb_msg_add_uint64(ldb, msg, msg, "uSNCreated", seq_num);
>  	if (ret != LDB_SUCCESS) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return ret;
>  	}
>  	ret = samdb_msg_add_uint64(ldb, msg, msg, "uSNChanged", seq_num);
>  	if (ret != LDB_SUCCESS) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return ret;
>  	}
>  	ret = ldb_msg_add_value(msg, "replPropertyMetaData", &nmd_value, NULL);
>  	if (ret != LDB_SUCCESS) {
>  		ldb_oom(ldb);
> +		talloc_free(ac);
>  		return ret;
>  	}
>  
> @@ -701,16 +719,19 @@ static int replmd_add(struct ldb_module *module, struct ldb_request *req)
>  				ac, replmd_op_callback,
>  				req);
>  	if (ret != LDB_SUCCESS) {
> +		talloc_free(ac);
>  		return ret;
>  	}
>  
>  	ret = replmd_notify(module, msg->dn, seq_num);
>  	if (ret != LDB_SUCCESS) {
> +		talloc_free(ac);
>  		return ret;
>  	}
>  
>         	/* if a control is there remove if from the modified request */
>  	if (control && !save_controls(control, down_req, &saved_controls)) {
> +		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
> @@ -818,7 +839,7 @@ static int replmd_update_rpmd(struct ldb_module *module,
>  
>  	/* search for the existing replPropertyMetaDataBlob */
>  	ret = dsdb_search_dn_with_deleted(ldb, msg, &res, msg->dn, attrs);
> -	if (ret != LDB_SUCCESS || res->count < 1) {
> +	if (ret != LDB_SUCCESS || res->count != 1) {
>  		DEBUG(0,(__location__ ": Object %s failed to find replPropertyMetaData\n",
>  			 ldb_dn_get_linearized(msg->dn)));
>  		return LDB_ERR_OPERATIONS_ERROR;
> @@ -913,9 +934,10 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
>  	const struct dsdb_schema *schema;
>  	struct ldb_request *down_req;
>  	struct ldb_message *msg;
> -	int ret;
> +	struct ldb_result *res;
>  	time_t t = time(NULL);
>  	uint64_t seq_num = 0;
> +	int ret;
>  
>  	/* do not manipulate our control entries */
>  	if (ldb_dn_is_special(req->op.mod.message->dn)) {
> @@ -944,13 +966,12 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
>  	/* we have to copy the message as the caller might have it as a const */
>  	msg = ldb_msg_copy_shallow(ac, req->op.mod.message);
>  	if (msg == NULL) {
> +		ldb_oom(ldb);
>  		talloc_free(ac);
>  		return LDB_ERR_OPERATIONS_ERROR;
>  	}
>  
>  	/* TODO:
> -	 * - get the whole old object
> -	 * - if the old object doesn't exist report an error
>  	 * - give an error when a readonly attribute should
>  	 *   be modified
>  	 * - merge the changed into the old object
> @@ -959,8 +980,15 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
>  	 *   attribute was changed
>  	 */
>  
> +	ret = dsdb_search_dn_with_deleted(ldb, msg, &res, msg->dn, NULL);
> +	if (ret != LDB_SUCCESS) {
> +		talloc_free(ac);
> +		return ret;
> +	}
> +
>  	ret = replmd_update_rpmd(module, msg, &seq_num);
>  	if (ret != LDB_SUCCESS) {
> +		talloc_free(ac);
>  		return ret;
>  	}
>  
> @@ -974,6 +1002,7 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
>  				ac, replmd_op_callback,
>  				req);
>  	if (ret != LDB_SUCCESS) {
> +		talloc_free(ac);
>  		return ret;
>  	}
>  	talloc_steal(down_req, msg);
> @@ -983,12 +1012,12 @@ static int replmd_modify(struct ldb_module *module, struct ldb_request *req)
>  	if (seq_num != 0) {
>  		if (add_time_element(msg, "whenChanged", t) != LDB_SUCCESS) {
>  			talloc_free(ac);
> -			return LDB_ERR_OPERATIONS_ERROR;
> +			return ret;
>  		}
>  
>  		if (add_uint64_element(msg, "uSNChanged", seq_num) != LDB_SUCCESS) {
>  			talloc_free(ac);
> -			return LDB_ERR_OPERATIONS_ERROR;
> +			return ret;
>  		}
>  	}
>  
> 
> 



More information about the samba-technical mailing list