[SCM] Samba Shared Repository - branch master updated

[Volker Lendecke]

The branch, master has been updated
       via  00513da g_lock: Simplify g_lock_trylock
       via  6b3cc79 g_lock: Avoid a double call to serverid_exist
      from  a98f09a selftest: Load time_audit and full_audit modules for all tests

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 00513daf78eb29996c81ca4d3be9e71a8e872829
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Aug 13 15:07:06 2018 +0200

    g_lock: Simplify g_lock_trylock
    
    While chasing a bug in g_lock (not in master) I saw some opportunity to
    simplify g_lock_trylock a bit. This is array handling, and array
    handling is just extremely error-prone. This *might* be a little less
    efficient or large numbers of READ locks, but this remains to be
    seen. For now, simplify the code.
    
    First, we make two passes now: One to remove ourselves, and the other
    one to search for conflicts. Mixing up both made it pretty hard for me
    to follow the code.
    
    Second, I've removed the _mylock and mylock pointer/struct logic and
    replaced it with the "mylock.pid.pid != 0 ? &mylock : NULL" when calling
    g_lock_store. To me, this focuses the logic whether to add ourselves in
    one place instead of spreading it around in the whole routine.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Tue Aug 14 11:42:10 CEST 2018 on sn-devel-144

commit 6b3cc7916b81e001b886b9c632babe345ca21d76
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Aug 13 14:12:47 2018 +0200

    g_lock: Avoid a double call to serverid_exist
    
    If we try to G_LOCK_READ while a G_LOCK_WRITE is active, we do the
    serverid_exists call twice. Avoid that.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/lib/g_lock.c | 56 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 19 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index bffbd6b..de24b6c 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -200,8 +200,7 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 	TDB_DATA data;
 	size_t i;
 	struct g_lock lck;
-	struct g_lock_rec _mylock;
-	struct g_lock_rec *mylock = NULL;
+	struct g_lock_rec mylock = {0};
 	NTSTATUS status;
 	bool modified = false;
 	bool ok;
@@ -226,15 +225,20 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 
 		g_lock_get_rec(&lck, i, &check_rec);
 
-		if (!serverid_exists(&check_rec.pid)) {
+		if ((check_rec.lock_type == G_LOCK_READ) &&
+		    !serverid_exists(&check_rec.pid)) {
 			g_lock_rec_del(&lck, i);
 			modified = true;
 		}
 	}
 
-	i = 0;
+	/*
+	 * For the lock upgrade/downgrade case, remove ourselves from
+	 * the list. We re-add ourselves later after we checked the
+	 * other entries for conflict.
+	 */
 
-	while (i < lck.num_recs) {
+	for (i=0; i<lck.num_recs; i++) {
 		struct g_lock_rec lock;
 
 		g_lock_get_rec(&lck, i, &lock);
@@ -244,20 +248,26 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 				status = NT_STATUS_WAS_LOCKED;
 				goto done;
 			}
-			if (mylock != NULL) {
-				status = NT_STATUS_INTERNAL_DB_CORRUPTION;
-				goto done;
-			}
-			_mylock = lock;
-			mylock = &_mylock;
-			/*
-			 * Remove "our" lock entry. Re-add it later
-			 * with our new lock type.
-			 */
+
+			mylock = lock;
 			g_lock_rec_del(&lck, i);
 			modified = true;
-			continue;
+			break;
 		}
+	}
+
+	/*
+	 * Check for conflicts with everybody else. Not a for-loop
+	 * because we remove stale entries in the meantime,
+	 * decrementing lck.num_recs.
+	 */
+
+	i = 0;
+
+	while (i < lck.num_recs) {
+		struct g_lock_rec lock;
+
+		g_lock_get_rec(&lck, i, &lock);
 
 		if (g_lock_conflicts(type, lock.lock_type)) {
 			struct server_id pid = lock.pid;
@@ -287,18 +297,26 @@ static NTSTATUS g_lock_trylock(struct db_record *rec, struct server_id self,
 
 	modified = true;
 
-	_mylock = (struct g_lock_rec) {
+	mylock = (struct g_lock_rec) {
 		.pid = self,
 		.lock_type = type
 	};
-	mylock = &_mylock;
 
 	status = NT_STATUS_OK;
 done:
 	if (modified) {
 		NTSTATUS store_status;
 
-		store_status = g_lock_store(rec, &lck, mylock);
+		/*
+		 * (Re-)add ourselves if needed via non-NULL
+		 * g_lock_store argument
+		 */
+
+		store_status = g_lock_store(
+			rec,
+			&lck,
+			mylock.pid.pid != 0 ? &mylock : NULL);
+
 		if (!NT_STATUS_IS_OK(store_status)) {
 			DBG_WARNING("g_lock_record_store failed: %s\n",
 				    nt_errstr(store_status));


-- 
Samba Shared Repository