On Wed, Aug 17, 2016 at 01:13:16PM +0200, Fabiano Fidêncio wrote: > This patchset resolves https://fedorahosted.org/sssd/ticket/3128 > > CI has passed: http://sssd-ci.duckdns.org/logs/job/51/84/summary.html > > Best Regards, > -- > Fabiano Fidêncio
> From fdef2d959af54e2208a3de86c1d3d7b03c58cce1 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> > Date: Tue, 16 Aug 2016 11:20:49 +0200 > Subject: [PATCH 1/2] SYSDB: Rework sysdb_cache_connect() ACK > From e4e232ea548119c140529857dd43dbebf31c627e Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]> > Date: Tue, 16 Aug 2016 11:46:41 +0200 > Subject: [PATCH 2/2] SYSDB: Remove the timestamp cache for a newly created The patch works fine, thank you. I just have one question related to code-style, see inline. > static errno_t sysdb_cache_connect_helper(TALLOC_CTX *mem_ctx, > + struct sysdb_ctx *sysdb, > struct sss_domain_info *domain, > const char *ldb_file, > int flags, > const char *exp_version, > const char *base_ldif, > + bool > remove_ts_cache_when_newly_created, > struct ldb_context **_ldb, > const char **_version) > { > @@ -527,6 +545,7 @@ static errno_t sysdb_cache_connect_helper(TALLOC_CTX > *mem_ctx, > const char *version = NULL; > int ret; > struct ldb_context *ldb; > + bool ldb_file_exists; > > tmp_ctx = talloc_new(NULL); > if (!tmp_ctx) { > @@ -534,6 +553,8 @@ static errno_t sysdb_cache_connect_helper(TALLOC_CTX > *mem_ctx, > goto done; > } > > + ldb_file_exists = !(access(ldb_file, F_OK) == -1 && errno == ENOENT); > + > ret = sysdb_ldb_connect(tmp_ctx, ldb_file, flags, &ldb); > if (ret != EOK) { > DEBUG(SSSDBG_CRIT_FAILURE, "sysdb_ldb_connect failed.\n"); > @@ -592,8 +613,18 @@ static errno_t sysdb_cache_connect_helper(TALLOC_CTX > *mem_ctx, > goto done; > } > > - /* The cache has been newly created. > - * We need to reopen the LDB to ensure that > + /* The cache has been newly created. */ > + if (remove_ts_cache_when_newly_created && !ldb_file_exists) { > + ret = remove_ts_cache(sysdb); > + if (ret != EOK) { > + DEBUG(SSSDBG_MINOR_FAILURE, > + "Could not delete the timestamp ldb file (%d) (%s)\n", > + ret, sss_strerror(ret)); > + goto done; > + } > + } > + Wouldn't it make more sense to move the logic outside to the caller (sysdb_cache_connect) ? that way the helper could just really help connect to database and the cache removal logic would be in the ldb-cache specific sysdb_cache_connect() function. _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
