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

On Tue, 23 Feb 2010 13:50:42 +0100 Jakub Hrozek jhrozek@redhat.com wrote:

•  = dp_opt_get_int(opts->basic,
  

•                                      SDAP_LOGIN_CACHE_TIMEOUT);
  

• if (!offline_credentials_expiration && ldap_cred_expiration) {

•    DEBUG(1, ("Conflicting values for options %s (unlimited) "
  

•              "and %s (%d)\n",
  

•              opts->basic[SDAP_LOGIN_CACHE_TIMEOUT].opt_name,
  

•              CONFDB_PAM_CRED_TIMEOUT,
  

•              offline_credentials_expiration));
  

•    ret = EINVAL;
  

•    goto done;
  

• }
• if (offline_credentials_expiration && ldap_cred_expiration &&

•    offline_credentials_expiration >= ldap_cred_expiration) {
  

•    DEBUG(1, ("Value of %s (now %d) must be larger "
  

•              "than value of %s (now %d)\n",
  

•              opts->basic[SDAP_LOGIN_CACHE_TIMEOUT].opt_name,
  

•              ldap_cred_expiration,
  

•              CONFDB_PAM_CRED_TIMEOUT,
  

•              offline_credentials_expiration));
  

•    ret = EINVAL;
  

•    goto done;
  

• }

This snipped is very confusing. It looks like ldap_cred_expiration should really be called login_cache_timeout, or what I am not understanding here?

Can you also add a comment that explains the checks ? Given the special case (0 == unlimited) I think a comment would help a lot the casual reader understanding what is going on here.

Simo.