[PATCH] lib: Fix gencache_del

[Volker Lendecke]

Hi!

Review appreciated!

Thanks, Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 3a3de8d8bd568f6d1dedfce34e91fcd82e23c940 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 3 Mar 2016 15:59:05 +0100
Subject: [PATCH] lib: Fix gencache_del

"net cache flush" can give nasty error messages like

Couldn't delete entry! key = IDMAP/UID2SID/12345

These happen when there's an already deleted entry in
gencache_notrans.tdb, indicated by a 0 timeout. This happens if two
gencache_del function calls have happened right after the other and a
gencache_stabilize has not wiped them.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/gencache.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index c353aa6..f3a0e12 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -370,6 +370,15 @@ done:
 	return ret == 0;
 }
 
+static void gencache_del_parser(time_t timeout, DATA_BLOB blob,
+				void *private_data)
+{
+	if (timeout != 0) {
+		bool *must_delete = private_data;
+		*must_delete = true;
+	}
+}
+
 /**
  * Delete one entry from the cache file.
  *
@@ -381,9 +390,10 @@ done:
 
 bool gencache_del(const char *keystr)
 {
-	bool exists, was_expired;
-	bool ret = false;
-	DATA_BLOB value;
+	TDB_DATA key = string_term_tdb_data(keystr);
+	bool must_delete = false;
+	bool result = true;
+	int ret;
 
 	if (keystr == NULL) {
 		return false;
@@ -395,28 +405,25 @@ bool gencache_del(const char *keystr)
 
 	DEBUG(10, ("Deleting cache entry (key=[%s])\n", keystr));
 
-	/*
-	 * We delete an element by setting its timeout to 0. This way we don't
-	 * have to do a transaction on gencache.tdb every time we delete an
-	 * element.
-	 */
+	ret = tdb_chainlock(cache_notrans->tdb, key);
+	if (ret == -1) {
+		return false;
+	}
 
-	exists = gencache_get_data_blob(keystr, NULL, &value, NULL,
-					&was_expired);
+	gencache_parse(keystr, gencache_del_parser, &must_delete);
 
-	if (!exists && was_expired) {
+	if (must_delete) {
 		/*
-		 * gencache_get_data_blob has implicitly deleted this
-		 * entry, so we have to return success here.
+		 * We delete an element by setting its timeout to
+		 * 0. This way we don't have to do a transaction on
+		 * gencache.tdb every time we delete an element.
 		 */
-		return true;
+		result = gencache_set(keystr, "", 0);
 	}
 
-	if (exists) {
-		data_blob_free(&value);
-		ret = gencache_set(keystr, "", 0);
-	}
-	return ret;
+	tdb_chainunlock(cache_notrans->tdb, key);
+
+	return result;
 }
 
 static bool gencache_pull_timeout(char *val, time_t *pres, char **pendptr)
-- 
2.1.4