Re: [SSSD] Behaviour of getgrnam/getgrgid

[Ralf Haferkamp]

Am Montag 27 September 2010, 16:37:14 schrieb Simo Sorce:
> On Fri, 24 Sep 2010 16:31:24 +0200
> 
> Ralf Haferkamp <rha...@suse.de> wrote:
> > Hi,
> > 
> > find updated patches attached. (Rebased against current master)
> > 
> > Am Donnerstag 23 September 2010, 20:02:20 schrieb Stephen Gallagher:
> > > On 09/20/2010 11:13 AM, Ralf Haferkamp wrote:
> > [..]
> > 
> > > Patch 0001: Ack. This looks fine to me.
> 
> Uhmm I think I see an issue in patch 1.
> 
> It looks to me that we fail the operation if we have no members in a
> group as when calling sysdb_attrs_get_el() sdap_save_group() now
> treats ENOENT just like a fatal error.
Hm, I am not sure what call to  sysdb_attrs_get_el() you are referring 
to, but if I understand the sysdb_attrs_get_el() code correctly it will 
never return ENOENT.
It just calls sysdb_attrs_get_el_int with alloc=true in that case the 
only error that can return is ENOMEN. Did I overlook something?
IIRC I explicitly tested the code with empty groups.

> Although uncommon I think we should handle empty groups.
Of course.

> Also I have a cosmetic request. It took sometimes to me to understand
> what the name sysdb_member_dns meant. Would it be possible to rename
> it to something like populate_members/resolve/members/fetch_members
> or something similar ? It would make it easier to understand it is an
> option that does something like store_members, just different.
Ok. Find updated patch attached.

-- 
Ralf

From bdd2b5877a0eeb0dbe0578390e4cd819776bbf2e Mon Sep 17 00:00:00 2001
From: Ralf Haferkamp <rha...@suse.de>
Date: Mon, 27 Sep 2010 21:33:41 +0200
Subject: [PATCH 1/2] Shortcut for save_group() to accept sysdb DNs as member attributes

Addtional parameter "populate_members" for save_group() and save_groups()
to indicate that the "member" attribute of the groups is populated with
sysdb DNs of the members (instead of LDAP DNs).
---
 src/providers/ldap/sdap_async_accounts.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_accounts.c b/src/providers/ldap/sdap_async_accounts.c
index 95f3b0d..b71e6d5 100644
--- a/src/providers/ldap/sdap_async_accounts.c
+++ b/src/providers/ldap/sdap_async_accounts.c
@@ -609,6 +609,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
                            struct sss_domain_info *dom,
                            struct sysdb_attrs *attrs,
                            bool store_members,
+                           bool populate_members,
                            char **_timestamp)
 {
     struct ldb_message_element *el;
@@ -697,7 +698,19 @@ static int sdap_save_group(TALLOC_CTX *memctx,
         }
     }
 
-    if (store_members) {
+    if (populate_members) {
+        struct ldb_message_element *el1;
+        ret = sysdb_attrs_get_el(attrs, opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el1);
+        if (ret != EOK) {
+            goto fail;
+        }
+        ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el);
+        if (ret != EOK) {
+            goto fail;
+        }
+        el->values = el1->values;
+        el->num_values = el1->num_values;
+    } else if (store_members) {
         ret = sysdb_attrs_get_el(attrs,
                         opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el);
         if (ret != EOK) {
@@ -808,6 +821,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
                             struct sdap_options *opts,
                             struct sysdb_attrs **groups,
                             int num_groups,
+                            bool populate_members,
                             char **_timestamp)
 {
     TALLOC_CTX *tmpctx;
@@ -848,7 +862,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
         /* if 2 pass savemembers = false */
         ret = sdap_save_group(tmpctx, sysdb,
                               opts, dom, groups[i],
-                              (!twopass), &timestamp);
+                              (!twopass), populate_members, &timestamp);
 
         /* Do not fail completely on errors.
          * Just report the failure to save and go on */
@@ -872,7 +886,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
         }
     }
 
-    if (twopass) {
+    if (twopass && !populate_members) {
 
         for (i = 0; i < num_groups; i++) {
 
@@ -988,6 +1002,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
     ret = sdap_save_groups(state, state->sysdb,
                            state->dom, state->opts,
                            state->groups, state->count,
+                           false,
                            &state->higher_timestamp);
     if (ret) {
         DEBUG(2, ("Failed to store groups.\n"));
@@ -1355,7 +1370,7 @@ static void sdap_initgr_nested_store(struct tevent_req *req)
     state = tevent_req_data(req, struct sdap_initgr_nested_state);
 
     ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
-                           state->groups, state->groups_cur, NULL);
+                           state->groups, state->groups_cur, false, NULL);
     if (ret) {
         tevent_req_error(req, ret);
         return;
-- 
1.7.1


_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel