On Tue, 23 Feb 2010 19:52:50 +0100 Jakub Hrozek jhrozek@redhat.com wrote:
- subfilter = talloc_asprintf(state, "(&(!(%s=0))(%s<=%ld))",
- account_cache_expiration =
dp_opt_get_int(state->ctx->opts->basic,
SDAP_ACCOUNT_CACHE_EXPIRATION);
- DEBUG(9, ("Cache expiration is set to %d days\n",
account_cache_expiration));- subfilter = talloc_asprintf(state,
"(&(!(%s=0))(%s<=%ld)", SYSDB_CACHE_EXPIRE,
SYSDB_CACHE_EXPIRE, (long)now);
SYSDB_CACHE_EXPIRE,(long) now);- if (!subfilter) {
DEBUG(2, ("Failed to build filter\n"));talloc_zfree(req);return NULL;- }
- if (account_cache_expiration > 0) {
subfilter = )subfilter,"(|(!(%s=*))(%s<=%ld)))",SYSDB_LAST_LOGIN,SYSDB_LAST_LOGIN,(long) (now -(account_cache_expiration * 86400)));
- } else {
subfilter = talloc_asprintf_append(subfilter,"(!(%s=*)))",SYSDB_LAST_LOGIN);- } if (!subfilter) { DEBUG(2, ("Failed to build filter\n")); talloc_zfree(req);
Given you are always calling talloc_asprintf_append() in either case, it would seem more efficient to simply have 2 completely separate talloc_asprintf() calls, even if that means repeating the same subset of arguments in the two branches of the if statement.
It took me a while to realize what was going on here (I didn't immediately realize these where _append()s )
Although this is not something I would consider nacking the patch for.
Simo.