Re: [SSSD] [PATCH] Better cleanup task handling

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.