[PATCH] a few cleanups

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Dec 14 16:14:52 UTC 2015


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 116eef572995df24d3dcd1d0bda6aff7fe5574fc Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 13 Dec 2015 15:14:18 +0100
Subject: [PATCH 1/6] lib: Add gencache.h

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/proto.h | 21 +-------------------
 source3/lib/gencache.h  | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 20 deletions(-)
 create mode 100644 source3/lib/gencache.h

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 7d0ba42..eeee402 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -88,26 +88,7 @@ int map_errno_from_nt_status(NTSTATUS status);
 
 struct file_id vfs_file_id_from_sbuf(connection_struct *conn, const SMB_STRUCT_STAT *sbuf);
 
-/* The following definitions come from lib/gencache.c  */
-
-bool gencache_set(const char *keystr, const char *value, time_t timeout);
-bool gencache_del(const char *keystr);
-bool gencache_get(const char *keystr, TALLOC_CTX *mem_ctx, char **value,
-		  time_t *ptimeout);
-bool gencache_parse(const char *keystr,
-		    void (*parser)(time_t timeout, DATA_BLOB blob,
-				   void *private_data),
-		    void *private_data);
-bool gencache_get_data_blob(const char *keystr, TALLOC_CTX *mem_ctx,
-			    DATA_BLOB *blob,
-			    time_t *timeout, bool *was_expired);
-bool gencache_stabilize(void);
-bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob, time_t timeout);
-void gencache_iterate_blobs(void (*fn)(const char *key, DATA_BLOB value,
-				       time_t timeout, void *private_data),
-			    void *private_data, const char *pattern);
-void gencache_iterate(void (*fn)(const char* key, const char *value, time_t timeout, void* dptr),
-                      void* data, const char* keystr_pattern);
+#include "lib/gencache.h"
 
 /* The following definitions come from lib/interface.c  */
 
diff --git a/source3/lib/gencache.h b/source3/lib/gencache.h
new file mode 100644
index 0000000..61716c4
--- /dev/null
+++ b/source3/lib/gencache.h
@@ -0,0 +1,53 @@
+/*
+ * Unix SMB/CIFS implementation.
+ *
+ * Generic, persistent and shared between processes cache mechanism for use
+ * by various parts of the Samba code
+ *
+ * Copyright (C) Rafal Szczesniak    2002
+ * Copyright (C) Volker Lendecke     2009
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __LIB_GENCACHE_H__
+#define __LIB_GENCACHE_H__
+
+#include "replace.h"
+#include "system/time.h"
+#include "lib/util/data_blob.h"
+#include "libcli/util/ntstatus.h"
+
+bool gencache_set(const char *keystr, const char *value, time_t timeout);
+bool gencache_del(const char *keystr);
+bool gencache_get(const char *keystr, TALLOC_CTX *mem_ctx, char **value,
+		  time_t *ptimeout);
+bool gencache_parse(const char *keystr,
+		    void (*parser)(time_t timeout, DATA_BLOB blob,
+				   void *private_data),
+		    void *private_data);
+bool gencache_get_data_blob(const char *keystr, TALLOC_CTX *mem_ctx,
+			    DATA_BLOB *blob,
+			    time_t *timeout, bool *was_expired);
+bool gencache_stabilize(void);
+bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
+			    time_t timeout);
+void gencache_iterate_blobs(void (*fn)(const char *key, DATA_BLOB value,
+				       time_t timeout, void *private_data),
+			    void *private_data, const char *pattern);
+void gencache_iterate(void (*fn)(const char* key, const char *value,
+				 time_t timeout, void* dptr),
+                      void* data, const char* keystr_pattern);
+
+#endif
-- 
1.9.1


From cf70198e227fe210150af12eaaee05f7def52b00 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 13 Dec 2015 15:17:27 +0100
Subject: [PATCH 2/6] gencache: True->true, False->false

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

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 90eafaa..ed78476 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -63,7 +63,9 @@ static bool gencache_init(void)
 	int open_flags = O_RDWR|O_CREAT;
 
 	/* skip file open if it's already opened */
-	if (cache) return True;
+	if (cache) {
+		return true;
+	}
 
 	cache_fname = cache_path("gencache.tdb");
 	if (cache_fname == NULL) {
@@ -112,7 +114,7 @@ static bool gencache_init(void)
 
 	if (!cache) {
 		DEBUG(5, ("Attempt to open gencache.tdb has failed.\n"));
-		return False;
+		return false;
 	}
 
 	cache_fname = lock_path("gencache_notrans.tdb");
@@ -139,7 +141,7 @@ static bool gencache_init(void)
 	}
 	TALLOC_FREE(cache_fname);
 
-	return True;
+	return true;
 }
 
 static TDB_DATA last_stabilize_key(void)
@@ -295,7 +297,9 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 		return false;
 	}
 
-	if (!gencache_init()) return False;
+	if (!gencache_init()) {
+		return false;
+	}
 
 	if (gencache_have_val(keystr, blob, timeout)) {
 		DEBUG(10, ("Did not store value for %s, we already got it\n",
@@ -305,7 +309,7 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 
 	val = talloc_asprintf(talloc_tos(), CACHE_DATA_FMT, (int)timeout);
 	if (val == NULL) {
-		return False;
+		return false;
 	}
 	val = talloc_realloc(NULL, val, char, talloc_array_length(val)-1);
 	if (val == NULL) {
@@ -383,7 +387,9 @@ bool gencache_del(const char *keystr)
 		return false;
 	}
 
-	if (!gencache_init()) return False;	
+	if (!gencache_init()) {
+		return false;
+	}
 
 	DEBUG(10, ("Deleting cache entry (key=[%s])\n", keystr));
 
@@ -569,7 +575,7 @@ static void gencache_get_data_blob_parser(time_t timeout, DATA_BLOB blob,
  *        timeout
  *
  * @retval true when entry is successfuly fetched
- * @retval False for failure
+ * @retval false for failure
  **/
 
 bool gencache_get_data_blob(const char *keystr, TALLOC_CTX *mem_ctx,
@@ -604,7 +610,7 @@ bool gencache_get_data_blob(const char *keystr, TALLOC_CTX *mem_ctx,
 		*timeout = state.timeout;
 	}
 
-	return True;
+	return true;
 
 fail:
 	if (was_expired != NULL) {
@@ -793,14 +799,14 @@ static int wipe_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
  *        timeout
  *
  * @retval true when entry is successfuly fetched
- * @retval False for failure
+ * @retval false for failure
  **/
 
 bool gencache_get(const char *keystr, TALLOC_CTX *mem_ctx, char **value,
 		  time_t *ptimeout)
 {
 	DATA_BLOB blob;
-	bool ret = False;
+	bool ret = false;
 
 	ret = gencache_get_data_blob(keystr, mem_ctx, &blob, ptimeout, NULL);
 	if (!ret) {
-- 
1.9.1


From 92876902dba945b6e6c62d26ab9a5b1bd446c072 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 13 Dec 2015 15:27:15 +0100
Subject: [PATCH 3/6] lib: Use directory_create_or_exist in xx_path

directory_create_or_exist is a little different: It does the lstat first and
sets the umask properly, but I think this is more correct than the xx_path()
version before.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/util.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/source3/lib/util.c b/source3/lib/util.c
index 1d04318..320fdc2 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1515,15 +1515,8 @@ static char *xx_path(const char *name, const char *rootpath)
 	}
 	trim_string(fname,"","/");
 
-	if (!directory_exist(fname)) {
-		if (mkdir(fname,0755) == -1) {
-			/* Did someone else win the race ? */
-			if (errno != EEXIST) {
-				DEBUG(1, ("Unable to create directory %s for file %s. "
-					"Error was %s\n", fname, name, strerror(errno)));
-				return NULL;
-			}
-		}
+	if (!directory_create_or_exist(fname, 0755)) {
+		return NULL;
 	}
 
 	return talloc_asprintf_append(fname, "/%s", name);
-- 
1.9.1


From 36e97a4ef0601bcc534c446838d1704af483627b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 13 Dec 2015 16:32:52 +0100
Subject: [PATCH 4/6] lib: Separate out xx_path() & callers

We should not have to #include proto.h just for cache_path() or so

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/proto.h |  4 +--
 source3/lib/util.c      | 65 ---------------------------------
 source3/lib/util_path.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++
 source3/lib/util_path.h | 31 ++++++++++++++++
 source3/wscript_build   |  1 +
 5 files changed, 128 insertions(+), 68 deletions(-)
 create mode 100644 source3/lib/util_path.c
 create mode 100644 source3/lib/util_path.h

diff --git a/source3/include/proto.h b/source3/include/proto.h
index eeee402..1470b6d 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -400,9 +400,7 @@ int smb_mkstemp(char *name_template);
 void *smb_xmalloc_array(size_t size, unsigned int count);
 char *myhostname(void);
 char *myhostname_upper(void);
-char *lock_path(const char *name);
-char *state_path(const char *name);
-char *cache_path(const char *name);
+#include "lib/util_path.h"
 bool parent_dirname(TALLOC_CTX *mem_ctx, const char *dir, char **parent,
 		    const char **name);
 bool ms_has_wild(const char *s);
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 320fdc2..303fa8b 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1496,71 +1496,6 @@ char *myhostname_upper(void)
 	return ret;
 }
 
-/**
- * @brief Returns an absolute path to a file concatenating the provided
- * @a rootpath and @a basename
- *
- * @param name Filename, relative to @a rootpath
- *
- * @retval Pointer to a string containing the full path.
- **/
-
-static char *xx_path(const char *name, const char *rootpath)
-{
-	char *fname = NULL;
-
-	fname = talloc_strdup(talloc_tos(), rootpath);
-	if (!fname) {
-		return NULL;
-	}
-	trim_string(fname,"","/");
-
-	if (!directory_create_or_exist(fname, 0755)) {
-		return NULL;
-	}
-
-	return talloc_asprintf_append(fname, "/%s", name);
-}
-
-/**
- * @brief Returns an absolute path to a file in the Samba lock directory.
- *
- * @param name File to find, relative to LOCKDIR.
- *
- * @retval Pointer to a talloc'ed string containing the full path.
- **/
-
-char *lock_path(const char *name)
-{
-	return xx_path(name, lp_lock_directory());
-}
-
-/**
- * @brief Returns an absolute path to a file in the Samba state directory.
- *
- * @param name File to find, relative to STATEDIR.
- *
- * @retval Pointer to a talloc'ed string containing the full path.
- **/
-
-char *state_path(const char *name)
-{
-	return xx_path(name, lp_state_directory());
-}
-
-/**
- * @brief Returns an absolute path to a file in the Samba cache directory.
- *
- * @param name File to find, relative to CACHEDIR.
- *
- * @retval Pointer to a talloc'ed string containing the full path.
- **/
-
-char *cache_path(const char *name)
-{
-	return xx_path(name, lp_cache_directory());
-}
-
 /*******************************************************************
  Given a filename - get its directory name
 ********************************************************************/
diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c
new file mode 100644
index 0000000..509ba5f
--- /dev/null
+++ b/source3/lib/util_path.c
@@ -0,0 +1,95 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Samba utility functions
+ * Copyright (C) Andrew Tridgell 1992-1998
+ * Copyright (C) Jeremy Allison 2001-2007
+ * Copyright (C) Simo Sorce 2001
+ * Copyright (C) Jim McDonough <jmcd at us.ibm.com> 2003
+ * Copyright (C) James Peach 2006
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "replace.h"
+#include <talloc.h>
+#include "lib/util/samba_util.h"
+#include "lib/util_path.h"
+
+struct share_params;
+#include "source3/param/param_proto.h"
+
+/**
+ * @brief Returns an absolute path to a file concatenating the provided
+ * @a rootpath and @a basename
+ *
+ * @param name Filename, relative to @a rootpath
+ *
+ * @retval Pointer to a string containing the full path.
+ **/
+
+static char *xx_path(const char *name, const char *rootpath)
+{
+	char *fname = NULL;
+
+	fname = talloc_strdup(talloc_tos(), rootpath);
+	if (!fname) {
+		return NULL;
+	}
+	trim_string(fname,"","/");
+
+	if (!directory_create_or_exist(fname, 0755)) {
+		return NULL;
+	}
+
+	return talloc_asprintf_append(fname, "/%s", name);
+}
+
+/**
+ * @brief Returns an absolute path to a file in the Samba lock directory.
+ *
+ * @param name File to find, relative to LOCKDIR.
+ *
+ * @retval Pointer to a talloc'ed string containing the full path.
+ **/
+
+char *lock_path(const char *name)
+{
+	return xx_path(name, lp_lock_directory());
+}
+
+/**
+ * @brief Returns an absolute path to a file in the Samba state directory.
+ *
+ * @param name File to find, relative to STATEDIR.
+ *
+ * @retval Pointer to a talloc'ed string containing the full path.
+ **/
+
+char *state_path(const char *name)
+{
+	return xx_path(name, lp_state_directory());
+}
+
+/**
+ * @brief Returns an absolute path to a file in the Samba cache directory.
+ *
+ * @param name File to find, relative to CACHEDIR.
+ *
+ * @retval Pointer to a talloc'ed string containing the full path.
+ **/
+
+char *cache_path(const char *name)
+{
+	return xx_path(name, lp_cache_directory());
+}
diff --git a/source3/lib/util_path.h b/source3/lib/util_path.h
new file mode 100644
index 0000000..118a4be
--- /dev/null
+++ b/source3/lib/util_path.h
@@ -0,0 +1,31 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Samba utility functions
+ * Copyright (C) Andrew Tridgell 1992-1998
+ * Copyright (C) Jeremy Allison 2001-2007
+ * Copyright (C) Simo Sorce 2001
+ * Copyright (C) Jim McDonough <jmcd at us.ibm.com> 2003
+ * Copyright (C) James Peach 2006
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __LIB_UTIL_PATH_H__
+#define __LIB_UTIL_PATH_H__
+
+char *lock_path(const char *name);
+char *state_path(const char *name);
+char *cache_path(const char *name);
+
+#endif
diff --git a/source3/wscript_build b/source3/wscript_build
index e66e89f..d40dd7e 100755
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -255,6 +255,7 @@ bld.SAMBA3_SUBSYSTEM('samba3util',
                    lib/util_sid.c
                    lib/util_file.c
                    lib/util.c
+                   lib/util_path.c
                    lib/util_procid.c
                    lib/util_sock.c
                    lib/util_tsock.c
-- 
1.9.1


From a09365c8dc586fe070a92c2539ca05b02da9767e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 13 Dec 2015 21:16:36 +0100
Subject: [PATCH 5/6] gencache: Refactor gencache_set_data_blob

Replace 3 calls into talloc with 1. Add an overflow check.

With this change, it will be easier to avoid the talloc call for small
blobs in the future and do it on the stack.

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

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index ed78476..ed16d79 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -283,6 +283,8 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 			    time_t timeout)
 {
 	int ret;
+	fstring hdr;
+	int hdr_len;
 	char* val;
 	time_t last_stabilize;
 	static int writecount;
@@ -307,19 +309,23 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 		return true;
 	}
 
-	val = talloc_asprintf(talloc_tos(), CACHE_DATA_FMT, (int)timeout);
-	if (val == NULL) {
+	hdr_len = fstr_sprintf(hdr, CACHE_DATA_FMT, (int)timeout);
+
+	if (hdr_len == -1) {
 		return false;
 	}
-	val = talloc_realloc(NULL, val, char, talloc_array_length(val)-1);
-	if (val == NULL) {
+	if ((blob->length + (size_t)hdr_len) < blob->length) {
 		return false;
 	}
-	val = (char *)talloc_append_blob(NULL, val, *blob);
+
+	val = talloc_array(talloc_tos(), char, hdr_len + blob->length);
 	if (val == NULL) {
 		return false;
 	}
 
+	memcpy(val, hdr, hdr_len);
+	memcpy(val+hdr_len, blob->data, blob->length);
+
 	DEBUG(10, ("Adding cache entry with key=[%s] and timeout="
 	           "[%s] (%d seconds %s)\n", keystr,
 		   timestring(talloc_tos(), timeout),
-- 
1.9.1


From f880856ba18050a6a99badab35d8feb44c481244 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 13 Dec 2015 21:21:47 +0100
Subject: [PATCH 6/6] lib: Remove unused talloc_append_blob

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/proto.h |  1 -
 source3/lib/util.c      | 26 --------------------------
 2 files changed, 27 deletions(-)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 1470b6d..4cd69fd 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -410,7 +410,6 @@ bool mask_match_search(const char *string, const char *pattern, bool is_case_sen
 bool mask_match_list(const char *string, char **list, int listLen, bool is_case_sensitive);
 bool unix_wild_match(const char *pattern, const char *string);
 bool name_to_fqdn(fstring fqdn, const char *name);
-void *talloc_append_blob(TALLOC_CTX *mem_ctx, void *buf, DATA_BLOB blob);
 uint32_t map_share_mode_to_deny_mode(uint32_t share_access, uint32_t private_options);
 
 #include "lib/util_procid.h"
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 303fa8b..2895c14 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1818,32 +1818,6 @@ bool name_to_fqdn(fstring fqdn, const char *name)
 	return true;
 }
 
-/**********************************************************************
- Append a DATA_BLOB to a talloc'ed object
-***********************************************************************/
-
-void *talloc_append_blob(TALLOC_CTX *mem_ctx, void *buf, DATA_BLOB blob)
-{
-	size_t old_size = 0;
-	char *result;
-
-	if (blob.length == 0) {
-		return buf;
-	}
-
-	if (buf != NULL) {
-		old_size = talloc_get_size(buf);
-	}
-
-	result = (char *)TALLOC_REALLOC(mem_ctx, buf, old_size + blob.length);
-	if (result == NULL) {
-		return NULL;
-	}
-
-	memcpy(result + old_size, blob.data, blob.length);
-	return result;
-}
-
 uint32_t map_share_mode_to_deny_mode(uint32_t share_access, uint32_t private_options)
 {
 	switch (share_access & ~FILE_SHARE_DELETE) {
-- 
1.9.1



More information about the samba-technical mailing list