[Freeipa-devel] [PATCH 0024] Add debug message to ldap_cache_addrdatalist()

[Adam Tkac]

On Tue, Jul 10, 2012 at 03:15:03PM +0200, Petr Spacek wrote:
> Hello,
> 
> this patch adds an debug message to ldap_cache_addrdatalist().
> 
> It is very useful for persistent search debugging.

Hi,

although idea of the patch is fine, I don't think that statements which allocate
memory should be in the "cleanup" path.

What about this (generally used in BIND):

ldap_cache_getrdatalist() {
...
char namebuf[DNS_NAME_FORMATSIZE];
...
cleanup:
    dns_name_format(name, namebuf, sizeof(namebuf));
    log_debug(20, "%s", namebuf);
}

Or you can put this entire code into "if (isc_log_debuglevel() >= 20)" statement
to save some CPU cycles...

Regards, Adam

> From 29a95bb7480802bfd9f10ccdffca6158eedf4581 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Thu, 28 Jun 2012 13:52:38 +0200
> Subject: [PATCH] Add debug message to ldap_cache_addrdatalist()
> 
> ---
>  src/cache.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/src/cache.c b/src/cache.c
> index c8afb99babd9de5263f0926ffd509e52fb87d8cd..2125d97bec2429307d3e0d16c31f9a2cc4bc1d00 100644
> --- a/src/cache.c
> +++ b/src/cache.c
> @@ -26,6 +26,7 @@
>  
>  #include <dns/rbt.h>
>  #include <dns/result.h>
> +#include <dns/log.h>
>  
>  #include <string.h>
>  
> @@ -207,6 +208,19 @@ ldap_cache_getrdatalist(isc_mem_t *mctx, ldap_cache_t *cache,
>  
>  cleanup:
>  	UNLOCK(&cache->mutex);
> +
> +	if (isc_log_getdebuglevel(dns_lctx) >= 20) {
> +		char *dns_str = NULL;
> +		if (dns_name_tostring(name, &dns_str, mctx) == ISC_R_SUCCESS) {
> +			log_debug(20, "cache search for '%s': %s", dns_str,
> +						isc_result_totext(result));
> +			isc_mem_free(mctx, dns_str);
> +		} else {
> +			log_debug(20, "cache search for '<DNS2STRERR>': %s",
> +						isc_result_totext(result));
> +		}
> +	}
> +
>  	return result;
>  }
>  
> -- 
> 1.7.7.6
> 


-- 
Adam Tkac, Red Hat, Inc.