This small series contain some really small refactoring work in the
save_user() and save_group() functions, as suggested by Lukaš
Slebodnik[0].
CI has not passed[1], but the failure occurred just in RHEL-7 in a
test that, AFAIR, is failing quite often. So, I'm not completely sure
whether it's related or not.
[0]:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/75ZRBKEDQVM75LUIDDLFDNLHUHJDZBNF/
[1]: http://sssd-ci.duckdns.org/logs/job/52/33/summary.html
Best Regards,
--
Fabiano Fidêncio
From ffc007cbc5479b60cb2a17556eea183bb9c9de15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 24 Aug 2016 13:16:31 +0200
Subject: [PATCH 1/5] PROXY: Remove lowercase attribute from save_user()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As this function already receives a struct sss_domain_info * parameter
as argument, we can simply check whether we will need a lowercase name
by accessing domain->case_sensitive.
Related:
https://fedorahosted.org/sssd/ticket/3134
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/providers/proxy/proxy_id.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index b0c8280..ff2631c 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -31,7 +31,7 @@
/* =Getpwnam-wrapper======================================================*/
static int save_user(struct sss_domain_info *domain,
- bool lowercase, struct passwd *pwd, const char *real_name,
+ struct passwd *pwd, const char *real_name,
const char *alias, uint64_t cache_timeout);
static int
@@ -143,8 +143,7 @@ static int get_pw_name(struct proxy_id_ctx *ctx,
}
/* Both lookups went fine, we can save the user now */
- ret = save_user(dom, !dom->case_sensitive, pwd,
- real_name, i_name, dom->user_timeout);
+ ret = save_user(dom, pwd, real_name, i_name, dom->user_timeout);
done:
talloc_zfree(tmpctx);
@@ -224,7 +223,7 @@ delete_user(struct sss_domain_info *domain,
}
static int save_user(struct sss_domain_info *domain,
- bool lowercase, struct passwd *pwd, const char *real_name,
+ struct passwd *pwd, const char *real_name,
const char *alias, uint64_t cache_timeout)
{
const char *shell;
@@ -246,7 +245,7 @@ static int save_user(struct sss_domain_info *domain,
gecos = NULL;
}
- if (lowercase || alias) {
+ if (!domain->case_sensitive || alias) {
attrs = sysdb_new_attrs(NULL);
if (!attrs) {
DEBUG(SSSDBG_CRIT_FAILURE, "Allocation error ?!\n");
@@ -255,7 +254,7 @@ static int save_user(struct sss_domain_info *domain,
}
}
- if (lowercase) {
+ if (!domain->case_sensitive) {
lc_pw_name = sss_tc_utf8_str_tolower(attrs, real_name);
if (lc_pw_name == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "Cannot convert name to lowercase.\n");
@@ -273,7 +272,7 @@ static int save_user(struct sss_domain_info *domain,
}
if (alias) {
- cased_alias = sss_get_cased_name(attrs, alias, !lowercase);
+ cased_alias = sss_get_cased_name(attrs, alias, domain->case_sensitive);
if (!cased_alias) {
ret = ENOMEM;
goto done;
@@ -366,8 +365,7 @@ static int get_pw_uid(struct proxy_id_ctx *ctx,
pwd->pw_name);
goto done;
}
- ret = save_user(dom, !dom->case_sensitive, pwd,
- name, NULL, dom->user_timeout);
+ ret = save_user(dom, pwd, name, NULL, dom->user_timeout);
done:
talloc_zfree(tmpctx);
@@ -497,8 +495,7 @@ static int enum_users(TALLOC_CTX *mem_ctx,
pwd->pw_name);
goto done;
}
- ret = save_user(dom, !dom->case_sensitive, pwd,
- name, NULL, dom->user_timeout);
+ ret = save_user(dom, pwd, name, NULL, dom->user_timeout);
if (ret) {
/* Do not fail completely on errors.
* Just report the failure to save and go on */
@@ -1331,8 +1328,7 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
goto done;
}
- ret = save_user(dom, !dom->case_sensitive, pwd,
- real_name, i_name, dom->user_timeout);
+ ret = save_user(dom, pwd, real_name, i_name, dom->user_timeout);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Could not save user\n");
goto fail;
--
2.7.4
From fe913fe51812f383846ffe084172e1254de03aca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 24 Aug 2016 13:25:44 +0200
Subject: [PATCH 2/5] PROXY: Remove cache_timeout attribute from save_user()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As this function already receives a struct sss_domain_info * parameter
as argument, we can simply get the cache_timeout attribute by accessing
domain->user_timeout.
Related:
https://fedorahosted.org/sssd/ticket/3134
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/providers/proxy/proxy_id.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index ff2631c..bdcac66 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -32,7 +32,7 @@
static int save_user(struct sss_domain_info *domain,
struct passwd *pwd, const char *real_name,
- const char *alias, uint64_t cache_timeout);
+ const char *alias);
static int
handle_getpw_result(enum nss_status status, struct passwd *pwd,
@@ -143,7 +143,7 @@ static int get_pw_name(struct proxy_id_ctx *ctx,
}
/* Both lookups went fine, we can save the user now */
- ret = save_user(dom, pwd, real_name, i_name, dom->user_timeout);
+ ret = save_user(dom, pwd, real_name, i_name);
done:
talloc_zfree(tmpctx);
@@ -224,7 +224,7 @@ delete_user(struct sss_domain_info *domain,
static int save_user(struct sss_domain_info *domain,
struct passwd *pwd, const char *real_name,
- const char *alias, uint64_t cache_timeout)
+ const char *alias)
{
const char *shell;
const char *gecos;
@@ -299,7 +299,7 @@ static int save_user(struct sss_domain_info *domain,
NULL,
attrs,
NULL,
- cache_timeout,
+ domain->user_timeout,
0);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Could not add user to cache\n");
@@ -365,7 +365,7 @@ static int get_pw_uid(struct proxy_id_ctx *ctx,
pwd->pw_name);
goto done;
}
- ret = save_user(dom, pwd, name, NULL, dom->user_timeout);
+ ret = save_user(dom, pwd, name, NULL);
done:
talloc_zfree(tmpctx);
@@ -495,7 +495,7 @@ static int enum_users(TALLOC_CTX *mem_ctx,
pwd->pw_name);
goto done;
}
- ret = save_user(dom, pwd, name, NULL, dom->user_timeout);
+ ret = save_user(dom, pwd, name, NULL);
if (ret) {
/* Do not fail completely on errors.
* Just report the failure to save and go on */
@@ -1328,7 +1328,7 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
goto done;
}
- ret = save_user(dom, pwd, real_name, i_name, dom->user_timeout);
+ ret = save_user(dom, pwd, real_name, i_name);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Could not save user\n");
goto fail;
--
2.7.4
From e8def9c0604f051473a6d57280df02b2297b09f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 24 Aug 2016 13:29:17 +0200
Subject: [PATCH 3/5] PROXY: Remove cache_timeout attribute from save_group()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As this function already receives a struct sss_domain_info * parameter
as argument, we can simply get the cache_timeout attribute by accessing
domain->group_timeout.
Related:
https://fedorahosted.org/sssd/ticket/3134
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/providers/proxy/proxy_id.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index bdcac66..a500a97 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -558,8 +558,7 @@ static errno_t proxy_process_missing_users(struct sysdb_ctx *sysdb,
static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
struct group *grp,
const char *real_name, /* already qualified */
- const char *alias, /* already qualified */
- uint64_t cache_timeout)
+ const char *alias) /* already qualified */
{
errno_t ret, sret;
struct sysdb_attrs *attrs = NULL;
@@ -664,7 +663,7 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
real_name,
grp->gr_gid,
attrs,
- cache_timeout,
+ dom->group_timeout,
now);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Could not add group to cache\n");
@@ -947,7 +946,7 @@ static int get_gr_name(struct proxy_id_ctx *ctx,
goto done;
}
- ret = save_group(sysdb, dom, grp, real_name, i_name, dom->group_timeout);
+ ret = save_group(sysdb, dom, grp, real_name, i_name);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannot save group [%d]: %s\n", ret, strerror(ret));
@@ -1032,7 +1031,7 @@ static int get_gr_gid(TALLOC_CTX *mem_ctx,
goto done;
}
- ret = save_group(sysdb, dom, grp, name, NULL, dom->group_timeout);
+ ret = save_group(sysdb, dom, grp, name, NULL);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannot save user [%d]: %s\n", ret, strerror(ret));
@@ -1165,8 +1164,7 @@ static int enum_groups(TALLOC_CTX *mem_ctx,
"Ignoring\n");
ret = ENOMEM;
}
- ret = save_group(sysdb, dom, grp, name,
- NULL, dom->group_timeout);
+ ret = save_group(sysdb, dom, grp, name, NULL);
if (ret) {
/* Do not fail completely on errors.
* Just report the failure to save and go on */
--
2.7.4
From 8727e4762dadddc17ffe943e741771b58f1e427a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 24 Aug 2016 13:32:10 +0200
Subject: [PATCH 4/5] PROXY: Mention that save_user()'s parameters are already
qualified
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Those comments are similar to what we have in the save_group() function.
Related:
https://fedorahosted.org/sssd/ticket/3134
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/providers/proxy/proxy_id.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index a500a97..7cae6df 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -223,8 +223,9 @@ delete_user(struct sss_domain_info *domain,
}
static int save_user(struct sss_domain_info *domain,
- struct passwd *pwd, const char *real_name,
- const char *alias)
+ struct passwd *pwd,
+ const char *real_name, /* already qualified */
+ const char *alias) /* already qualified */
{
const char *shell;
const char *gecos;
--
2.7.4
From 9f92f7b861f4f82f00dd231078a64cc23bc40720 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 24 Aug 2016 14:28:42 +0200
Subject: [PATCH 5/5] PROXY: Share common code of save_{group,user}()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
These two functions (save_user() and save_group()) share, between
themselves, the code preparing the attributes that are going to be
stored in the sysdb.
This patch basically splits this code out of those functions and
introduces the new prepare_attrs_for_saving_ops().
Related:
https://fedorahosted.org/sssd/ticket/3134
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/providers/proxy/proxy_id.c | 142 ++++++++++++++++++-----------------------
1 file changed, 62 insertions(+), 80 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index 7cae6df..a2782cf 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -222,49 +222,37 @@ delete_user(struct sss_domain_info *domain,
return ret;
}
-static int save_user(struct sss_domain_info *domain,
- struct passwd *pwd,
- const char *real_name, /* already qualified */
- const char *alias) /* already qualified */
+static int prepare_attrs_for_saving_ops(TALLOC_CTX *tmp_ctx,
+ bool case_sentitive,
+ const char *real_name, /* already_qualified */
+ const char *alias, /* already qualified */
+ struct sysdb_attrs **attrs)
{
- const char *shell;
- const char *gecos;
- struct sysdb_attrs *attrs = NULL;
- errno_t ret;
- const char *cased_alias;
- const char *lc_pw_name = NULL;
-
- if (pwd->pw_shell && pwd->pw_shell[0] != '\0') {
- shell = pwd->pw_shell;
- } else {
- shell = NULL;
- }
-
- if (pwd->pw_gecos && pwd->pw_gecos[0] != '\0') {
- gecos = pwd->pw_gecos;
- } else {
- gecos = NULL;
- }
-
- if (!domain->case_sensitive || alias) {
- attrs = sysdb_new_attrs(NULL);
- if (!attrs) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Allocation error ?!\n");
- ret = ENOMEM;
- goto done;
+ const char *lc_name = NULL;
+ const char *cased_alias = NULL;
+ errno_t ret = EOK;
+
+ if (!case_sentitive || alias == NULL) {
+ if (*attrs == NULL) {
+ *attrs = sysdb_new_attrs(tmp_ctx);
+ if (*attrs == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Allocation error ?!\n");
+ ret = ENOMEM;
+ goto done;
+ }
}
}
- if (!domain->case_sensitive) {
- lc_pw_name = sss_tc_utf8_str_tolower(attrs, real_name);
- if (lc_pw_name == NULL) {
+ if (!case_sentitive) {
+ lc_name = sss_tc_utf8_str_tolower(*attrs, real_name);
+ if (lc_name == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "Cannot convert name to lowercase.\n");
ret = ENOMEM;
goto done;
}
- ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lc_pw_name);
- if (ret) {
+ ret = sysdb_attrs_add_string(*attrs, SYSDB_NAME_ALIAS, lc_name);
+ if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
ret = ENOMEM;
goto done;
@@ -273,22 +261,54 @@ static int save_user(struct sss_domain_info *domain,
}
if (alias) {
- cased_alias = sss_get_cased_name(attrs, alias, domain->case_sensitive);
+ cased_alias = sss_get_cased_name(*attrs, alias, case_sentitive);
if (!cased_alias) {
ret = ENOMEM;
goto done;
}
/* Add the alias only if it differs from lowercased pw_name */
- if (lc_pw_name == NULL || strcmp(cased_alias, lc_pw_name) != 0) {
- ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, cased_alias);
- if (ret) {
+ if (lc_name == NULL || strcmp(cased_alias, lc_name) != 0) {
+ ret = sysdb_attrs_add_string(*attrs, SYSDB_NAME_ALIAS, cased_alias);
+ if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
goto done;
}
}
}
+done:
+ return ret;
+}
+
+static int save_user(struct sss_domain_info *domain,
+ struct passwd *pwd,
+ const char *real_name, /* already qualified */
+ const char *alias) /* already qualified */
+{
+ const char *shell;
+ const char *gecos;
+ struct sysdb_attrs *attrs = NULL;
+ errno_t ret;
+
+ if (pwd->pw_shell && pwd->pw_shell[0] != '\0') {
+ shell = pwd->pw_shell;
+ } else {
+ shell = NULL;
+ }
+
+ if (pwd->pw_gecos && pwd->pw_gecos[0] != '\0') {
+ gecos = pwd->pw_gecos;
+ } else {
+ gecos = NULL;
+ }
+
+ ret = prepare_attrs_for_saving_ops(NULL, !domain->case_sensitive,
+ real_name, alias, &attrs);
+ if (ret != EOK) {
+ goto done;
+ }
+
ret = sysdb_store_user(domain,
real_name,
pwd->pw_passwd,
@@ -563,8 +583,6 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
{
errno_t ret, sret;
struct sysdb_attrs *attrs = NULL;
- const char *cased_alias;
- const char *lc_gr_name = NULL;
TALLOC_CTX *tmp_ctx;
time_t now = time(NULL);
bool in_transaction = false;
@@ -618,46 +636,10 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
}
}
- if (dom->case_sensitive == false || alias) {
- if (!attrs) {
- attrs = sysdb_new_attrs(tmp_ctx);
- if (!attrs) {
- DEBUG(SSSDBG_CRIT_FAILURE, "Allocation error ?!\n");
- ret = ENOMEM;
- goto done;
- }
- }
- }
-
- if (dom->case_sensitive == false) {
- lc_gr_name = sss_tc_utf8_str_tolower(attrs, real_name);
- if (lc_gr_name == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "Cannot convert name to lowercase.\n");
- ret = ENOMEM;
- goto done;
- }
-
- ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, lc_gr_name);
- if (ret != EOK) {
- goto done;
- }
- }
-
- if (alias) {
- cased_alias = sss_get_cased_name(attrs, alias, dom->case_sensitive);
- if (!cased_alias) {
- ret = ENOMEM;
- DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
- goto done;
- }
-
- if (lc_gr_name == NULL || strcmp(cased_alias, lc_gr_name)) {
- ret = sysdb_attrs_add_string(attrs, SYSDB_NAME_ALIAS, cased_alias);
- if (ret) {
- DEBUG(SSSDBG_OP_FAILURE, "Could not add name alias\n");
- goto done;
- }
- }
+ ret = prepare_attrs_for_saving_ops(tmp_ctx, !dom->case_sensitive,
+ real_name, alias, &attrs);
+ if (ret != EOK) {
+ goto done;
}
ret = sysdb_store_group(dom,
--
2.7.4
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
