search

cgrzemba / Posix-Winsync-Plugin-for-389-directory-server

This Plugin syncs Posix attributes between Directoryserver and MS AD
Other
3 stars 2 forks source link

incorrect memberUidLock #8

Closed richm closed 12 years ago

richm commented 12 years ago

This code is not correct if(posix_winsync_config_get_mapMemberUid()) memberUidLock(); modGroupMembership(ds_entry,smods,do_modify); memberUidUnlock();

If posix_winsync_config_get_mapMemberUid() is FALSE then the code will call Unlock without first having called Lock. This is usually ok, but could lead to problems. To be correct, should call Lock/Unlock in pairs

richm commented 12 years ago

patch - http://rmeggins.fedorapeople.org/0001-issue-8-incorrect-memberUidLock.patch

cgrzemba commented 12 years ago

That's not what should happen. The error is the missing brace for the if- statement:

diff --git a/posix-winsync.c b/posix-winsync.c
index 95b1d67..739b3eb 100644
--- a/posix-winsync.c
+++ b/posix-winsync.c
@@ -920,10 +920,11 @@ posix_winsync_pre_ds_mod_group_cb(void *cbdata, const Slap        }
     slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
                     "_pre_ds_mod_group_cb present %d modify %d before\n",is_pre-       if(posix_winsync_config_get_mapMemberUid())
+       if(posix_winsync_config_get_mapMemberUid()){
            memberUidLock();
         modGroupMembership(ds_entry,smods,do_modify);
         memberUidUnlock();
+    }

     slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
                     "_pre_ds_mod_group_cb present %d modify %d\n",is_present_lo@@ -1082,10 +1083,11 @@ posix_winsync_pre_ds_add_group_cb(void *cbdata, const Sl             slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
                     "<-- _pre_ds_add_group_cb -- adding objectclass for new ent         }else{
-            if(posix_winsync_config_get_mapMemberUid())
+            if(posix_winsync_config_get_mapMemberUid()){
                 memberUidLock();
                 addGroupMembership(ds_entry,ad_entry);
                 memberUidUnlock();
+            }
         }
     }
     slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
richm commented 12 years ago

Yes, I figured that out. I cloned your repo and pushed my patch to my clone. If you look at that patch, you'll see that it correctly locks and unlocks.