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?= <fiden...@redhat.com>
> 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?= <fiden...@redhat.com>
> 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
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
