On Mon, Oct 11, 2010 at 02:32:59PM -0400, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 10/05/2010 10:21 AM, Sumit Bose wrote:
> Hi,
>
> this series of patches continue the work Stephen has started in
> "[PATCHES] Support for netgroups in the NSS client and responder".
>
> We decided to try to be as compatible to nss_ldap as possible, i.e. we
> do not any group unrolling or loop detection inside of sssd, but rely on
> glibc. To achieve this I added support to return netgroup member groups
> to the client and glibc. This is mostly done in 0003 and 0006. 0007 and
> 0008 add the necessary support to the LDAP provider.
>
> There is one difference to nss_ldap. If a netgroup member is not
> specified by a plain name but by a DN nss_ldap just returns the DN
> string to glibc and then glibc searches for a netgroup where the name is
> the returned DN. Even nss_ldap cannot find a matching netgroup for this
> name. If sssd detects a DN in the member list it tries to translate it
> to the corresponding name of the netgroup. If this fails it will return
> the full DN.
>
> The patches 0001 and 0002 fixes two errors in Stephen's patches.
Patch 0001: Nack. Please reverse the order of these two functions. The
request should be set to "done" before we post it.
done
Patch 0002: Ack
Patch 0003: Please update the comment about the wire protocol to mention
that byte 8 describes the 8-bit value for the reply type.
done
Also, I'm not sure that this is safe for aligned platforms like MIPS. It
might be wisest to just make this a 32-bit value and use the alignment
code here.
Currently it is handled as char/uint8_t, so it shouldn't be an issue. But
if you prefer a uint32_t here I can change it. One other reason to
change it would be that we always use uint32_t for such things in the
wire protocol. What do you think?
Patch 0004: Ack
Patch 0005: Ack
Patch 0006: To avoid doing two calls to talloc_realloc() (which can be
expensive, especially for very large netgroups) it might be best to do
both element checks first and realloc with the results for both ahead of
populating them.
done
In lookup_netgr_step() you're still using &netgr->triples as the return
variable. I think you were correct elsewhere to change this to entries,
and the same should be done here.
done
Also, I'm not sure if we want to return EINVAL for an empty
member. It's
probably enough to just skip it.
done
Patch 0007: Ack
Patch 0008: Nack
netgr_translate_members_send():
After calling sysdb_search_entry, please free sysdb_filter and
netgr_basedn since we won't need them again.
Spelling mistake: all_resovled should be all_resolved. Also in
update_dn_list()
done
You can't return from netgr_translate_members_ldap_step() with
tevent_req_done() (such as when we hit ENOMEM) safely.
If it happens on the first call (from within
netgr_translate_members_send()) then the callback will never fire. You
should change netgr_translate_members_ldap_step() to return errno_t types.
EOK should mean immediately call tevent_req_done() and
tevent_req_post(). If it returns EAGAIN, reenter the mainloop. Any other
error should call tevent_req_error() and tevent_req_post().
good catch, done
Please steal cn_attr onto the subreq so that it's cleaned up once
the
subreq is freed in netgr_translate_members_ldap_done(). Otherwise, we're
carrying an extra cn_attrs array around in memory for every nested
netgroup we're processing.
done
Patch 0009: Ack
New versions attached.
bye,
Sumit
- --
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAkyzWFsACgkQeiVVYja6o6MtXQCfcy91N7g1iINZ5Ss8/NpqE+uU
lOAAnRZ1xnLiNNzhHtaijS7axY/396eQ
=h8/P
-----END PGP SIGNATURE-----
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel