[Freeipa-devel] [PATCH 0049] Fix two memory leaks in persistent search

[Adam Tkac]

On Tue, Aug 14, 2012 at 02:32:55PM +0200, Petr Spacek wrote:
> Hello,
> 
> This patch fixes two memory leaks in persistent search.

Ack.

> From 892f1d5c59a97cdad7a2807ecd172488605ab181 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Tue, 14 Aug 2012 12:38:43 +0200
> Subject: [PATCH] Fix two memory leaks in persistent search.
> 
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
>  src/ldap_helper.c | 45 ++++++++++++---------------------------------
>  1 file changed, 12 insertions(+), 33 deletions(-)
> 
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index cc7003a6cdcd2d27404fec936623ed8a3e8fa7f8..6f9711c55abc008a1abfd4b36382bdf697cd2bd8 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -313,8 +313,7 @@ static isc_result_t ldap_pool_connect(ldap_pool_t *pool,
>  		ldap_instance_t *ldap_inst);
>  
>  /* Functions for manipulating LDAP persistent search control */
> -static isc_result_t ldap_pscontrol_create(isc_mem_t *mctx, LDAPControl **ctrlp);
> -static void ldap_pscontrol_destroy(isc_mem_t *mctx, LDAPControl **ctrlp);
> +static isc_result_t ldap_pscontrol_create(LDAPControl **ctrlp);
>  
>  static isc_threadresult_t ldap_psearch_watcher(isc_threadarg_t arg);
>  static void psearch_update(ldap_instance_t *inst, ldap_entry_t *entry,
> @@ -639,8 +638,7 @@ new_ldap_connection(ldap_pool_t *pool, ldap_connection_t **ldap_connp)
>  
>  	CHECK(isc_lex_create(ldap_conn->mctx, TOKENSIZ, &ldap_conn->lex));
>  	CHECKED_MEM_GET(ldap_conn->mctx, ldap_conn->rdata_target_mem, MINTSIZ);
> -	CHECK(ldap_pscontrol_create(ldap_conn->mctx,
> -				    &ldap_conn->serverctrls[0]));
> +	CHECK(ldap_pscontrol_create(&ldap_conn->serverctrls[0]));
>  
>  	*ldap_connp = ldap_conn;
>  
> @@ -674,8 +672,7 @@ destroy_ldap_connection(ldap_pool_t *pool, ldap_connection_t **ldap_connp)
>  			    ldap_conn->rdata_target_mem, MINTSIZ);
>  	}
>  	if (ldap_conn->serverctrls[0] != NULL) {
> -		ldap_pscontrol_destroy(ldap_conn->mctx,
> -				       &ldap_conn->serverctrls[0]);
> +		ldap_control_free(ldap_conn->serverctrls[0]);
>  	}
>  
>  	isc_mem_detach(&ldap_conn->mctx);
> @@ -2819,10 +2816,10 @@ cleanup:
>   * http://tools.ietf.org/id/draft-ietf-ldapext-psearch-03.txt) control.
>   */
>  static isc_result_t
> -ldap_pscontrol_create(isc_mem_t *mctx, LDAPControl **ctrlp)
> +ldap_pscontrol_create(LDAPControl **ctrlp)
>  {
>  	BerElement *ber;
> -	LDAPControl *ctrl = NULL;
> +	struct berval *berval;
>  	isc_result_t result = ISC_R_FAILURE;
>  
>  	REQUIRE(ctrlp != NULL && *ctrlp == NULL);
> @@ -2841,45 +2838,25 @@ ldap_pscontrol_create(isc_mem_t *mctx, LDAPControl **ctrlp)
>  	if (ber_printf(ber, "{ibb}", LDAP_ENTRYCHANGE_ALL, 0, 1) == -1)
>  		goto cleanup;
>  
> -	CHECKED_MEM_GET(mctx, ctrl, sizeof(*ctrl));
> -	ZERO_PTR(ctrl);
> -	ctrl->ldctl_iscritical = 1;
> -	ctrl->ldctl_oid = strdup(LDAP_CONTROL_PERSISTENTSEARCH);
> -	if (ctrl->ldctl_oid == NULL)
> +	if (ber_flatten(ber, &berval) < 0)
>  		goto cleanup;
>  
> -	if (ber_flatten2(ber, &ctrl->ldctl_value, 1) < 0)
> +	if (ldap_control_create(LDAP_CONTROL_PERSISTENTSEARCH, 1, berval, 1, ctrlp)
> +			!= LDAP_SUCCESS)
>  		goto cleanup;
>  
>  	ber_free(ber, 1);
> -	*ctrlp = ctrl;
> +	ber_bvfree(berval);
>  
>  	return ISC_R_SUCCESS;
>  
>  cleanup:
>  	ber_free(ber, 1);
> -	ldap_pscontrol_destroy(mctx, &ctrl);
> +	ber_bvfree(berval);
>  
>  	return result;
>  }
>  
> -static void
> -ldap_pscontrol_destroy(isc_mem_t *mctx, LDAPControl **ctrlp)
> -{
> -	LDAPControl *ctrl;
> -
> -	REQUIRE(ctrlp != NULL);
> -
> -	if (*ctrlp == NULL)
> -		return;
> -
> -	ctrl = *ctrlp;
> -	if (ctrl->ldctl_oid != NULL)
> -		free(ctrl->ldctl_oid);
> -	SAFE_MEM_PUT(mctx, ctrl, sizeof(*ctrl));
> -	*ctrlp = NULL;
> -}
> -
>  static inline isc_result_t
>  ldap_get_zone_serial(ldap_instance_t *inst, dns_name_t *zone_name,
>  				isc_uint32_t *serial) {
> @@ -3357,6 +3334,8 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
>  	isc_task_send(inst->task, (isc_event_t **)&pevent);
>  
>  cleanup:
> +	if (ctrls != NULL)
> +		ldap_controls_free(ctrls);
>  	if (result != ISC_R_SUCCESS) {
>  		if (dbname != NULL)
>  			isc_mem_free(mctx, dbname);
> -- 
> 1.7.11.2
> 


-- 
Adam Tkac, Red Hat, Inc.