On Mon, Nov 19, 2012 at 05:51:53PM +0100, Pavel Březina wrote:
On 11/18/2012 11:58 PM, Jakub Hrozek wrote:
>[PATCH 1/2] SYSDB: Use the add_string convenience functions for managing
>ghost user attribute
>
>Using the convenience function instead of low-level ldb calls makes the
>code more compact and more readable.
>
>[PATCH 2/2] LDAP: Only convert direct parents' ghost attribute to member
>
>https://fedorahosted.org/sssd/ticket/1612
>
>I'm not too happy with the originalDN attribute exposed in the
>sysdb_add_user/store_user functions but it seemed like a better way
>than grabbing the attribute from the extended attributes in the "attrs"
>array. At the same time, we need to adjust the attributes during all user
>adds, not just during sdap_store_user from the LDAP provider. Ideas on
>how to make the process cleaner are welcome.
>
>This patch changes the handling of ghost attributes when saving the
>actual user entry. Instead of always linking all groups that contained
>the ghost attribute with the new user entry, the original member
>attributes are now saved in the group object and the user entry is only
>linked with its direct parents.
>
>As the member attribute is compared against the originalDN of the user,
>if either the originalDN or the originalMember attributes are missing,
>the user object is linked with all the groups as a fallback.
>
>The original member attributes are only saved if the LDAP schema
>supports nesting.
>@@ -235,6 +236,19 @@ sdap_process_ghost_members(struct sysdb_attrs *attrs,
> return ret;
> }
>
Hi,
just one nitpick.
>+ if (store_original_member) {
>+ DEBUG(SSSDBG_TRACE_FUNC, ("The group has %d members\n",
memberel->num_values));
>+ for (i = 0; i < memberel->num_values; i++) {
>+ ret = sysdb_attrs_add_string(sysdb_attrs, SYSDB_ORIG_MEMBER,
>+ (const char *)
memberel->values[i].data);
>+ if (ret) {
>+ DEBUG(SSSDBG_OP_FAILURE, ("Could not add member [%s]\n",
>+ (const char *) memberel->values[i].data));
wrong indentation^^
>+ return ret;
>+ }
>+ }
>+ }
>+
Otherwise it looks good to me.
Thank you for the review. Simo requested some additional changes, in
particular:
* use "it" instead of "him" when talking about user
* don't misuse twopass, but use a separate variable
* fix code to adhere to 80-columns rule on several places
New patches are attached.