On 11/23/2015 05:35 PM, Pavel Reichl wrote:
On 11/17/2015 11:51 AM, Sumit Bose wrote:
From 14ad252d2715e5eb1eed139d4497fc0155c51f85 Mon Sep 17 00:00:00 2001
From: Sumit Bose<sb...@redhat.com>
Date: Fri, 13 Nov 2015 12:15:52 +0100
Subject: [PATCH 2/2] initgr: only search for primary group if it is not
already cached
Related tohttps://fedorahosted.org/sssd/ticket/2868
---
src/providers/ldap/sdap_async_initgroups.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/src/providers/ldap/sdap_async_initgroups.c
b/src/providers/ldap/sdap_async_initgroups.c
index
e9fcb272d3e906555c63d51e0aab90d1f575f3f8..2c25505753c0705649a70bb7ee3ab0dd42d2c944
100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -3065,6 +3065,7 @@ static void sdap_get_initgr_done(struct tevent_req
*subreq)
char *dom_sid_str;
char *group_sid_str;
struct sdap_options *opts = state->opts;
+ struct ldb_message *msg;
DEBUG(SSSDBG_TRACE_ALL, "Initgroups done\n");
@@ -3182,14 +3183,25 @@ static void sdap_get_initgr_done(struct tevent_req
*subreq)
goto fail;
}
- subreq = groups_get_send(req, state->ev, state->id_ctx,
- state->id_ctx->opts->sdom, state->conn,
- gid, BE_FILTER_IDNUM, BE_ATTR_ALL, false, false);
- if (!subreq) {
- ret = ENOMEM;
- goto fail;
+ ret = sysdb_search_group_by_gid(tmp_ctx, state->dom, primary_gid, NULL,
+ &msg);
+ if (ret == EOK) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ "Primary group already cached, nothing to do.\n");
+ talloc_free(tmp_ctx);
Do you think it would be bad idea to remove this line
+ tevent_req_done(req);
+ return;
and this?
I think we can reuse 'free' and 'return' that is executed right before 'fail'
namespace. It would be nice to avoid adding extra return.
+ } else {
would you also consider moving
gid = talloc_asprintf(state, "%lu", (unsigned long)primary_gid);
if (gid == NULL) {
ret = ENOMEM;
goto fail;
}
into this branch only?
+ subreq = groups_get_send(req, state->ev, state->id_ctx,
+ state->id_ctx->opts->sdom, state->conn,
+ gid, BE_FILTER_IDNUM, BE_ATTR_ALL, false,
+ false);
+ if (!subreq) {
+ ret = ENOMEM;
+ goto fail;
+ }
+ tevent_req_set_callback(subreq, sdap_get_initgr_pgid, req);
}
- tevent_req_set_callback(subreq, sdap_get_initgr_pgid, req);
talloc_free(tmp_ctx);
return;
-- 2.1.0
Hello I'm just starting to review the patch. I haven't tested the code yet but
it looks OK to me and CI passed. I have just a little proposal mentioned above.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
