s3-lib Remove the clobber_region() code.

This code wrote to the full buffer in fstrcpy(), pstrcpy() and other
fixed-length string manipulation functions.

The hope of this code was to find out at run time if we were mixing up
pstring and fstring etc, and to record where this came from.  It has a
runtime performance impact (particularly if compiled with
--enable-developer).

It is being removed because of the complexity it adds, and the
distinct lack of bugs that this complexity has been credited in
finding.

The macro-based compile-time checking of string sizes remains.

Andrew Bartlett

5 files changed, 37 insertions, 197 deletions

diff --git a/source3/lib/charcnv.c b/source3/lib/charcnv.c
index 5b2149b9c1..1779c4fbf7 100644
--- a/source3/lib/charcnv.c
+++ b/source3/lib/charcnv.c
@@ -1297,11 +1297,11 @@ size_t pull_ucs2(const void *base_ptr, char *dest, const void *src, size_t dest_
 **/
 
 size_t pull_ucs2_base_talloc(TALLOC_CTX *ctx,
-			const void *base_ptr,
-			char **ppdest,
-			const void *src,
-			size_t src_len,
-			int flags)
+			     const void *base_ptr,
+			     char **ppdest,
+			     const void *src,
+			     size_t src_len,
+			     int flags)
 {
 	char *dest;
 	size_t dest_len;
@@ -1476,24 +1476,9 @@ bool pull_ascii_talloc(TALLOC_CTX *ctx, char **dest, const char *src,
  is -1 then no maxiumum is used.
 **/
 
-size_t push_string_check_fn(const char *function, unsigned int line,
-			    void *dest, const char *src,
-			    size_t dest_len, int flags)
+size_t push_string_check_fn(void *dest, const char *src,
+			 size_t dest_len, int flags)
 {
-#ifdef DEVELOPER
-	/* We really need to zero fill here, not clobber
-	 * region, as we want to ensure that valgrind thinks
-	 * all of the outgoing buffer has been written to
-	 * so a send() or write() won't trap an error.
-	 * JRA.
-	 */
-#if 0
-	clobber_region(function, line, dest, dest_len);
-#else
-	memset(dest, '\0', dest_len);
-#endif
-#endif
-
 	if (!(flags & STR_ASCII) && (flags & STR_UNICODE)) {
 		return push_ucs2(NULL, dest, src, dest_len, flags);
 	}
@@ -1515,24 +1500,10 @@ size_t push_string_check_fn(const char *function, unsigned int line,
  is -1 then no maxiumum is used.
 **/
 
-size_t push_string_base(const char *function, unsigned int line,
-			const char *base, uint16 flags2, 
+size_t push_string_base(const char *base, uint16 flags2,
 			void *dest, const char *src,
 			size_t dest_len, int flags)
 {
-#ifdef DEVELOPER
-	/* We really need to zero fill here, not clobber
-	 * region, as we want to ensure that valgrind thinks
-	 * all of the outgoing buffer has been written to
-	 * so a send() or write() won't trap an error.
-	 * JRA.
-	 */
-#if 0
-	clobber_region(function, line, dest, dest_len);
-#else
-	memset(dest, '\0', dest_len);
-#endif
-#endif
 
 	if (!(flags & STR_ASCII) && \
 	    ((flags & STR_UNICODE || \
@@ -1559,15 +1530,6 @@ size_t push_string_base(const char *function, unsigned int line,
 ssize_t push_string(void *dest, const char *src, size_t dest_len, int flags)
 {
 	size_t ret;
-#ifdef DEVELOPER
-	/* We really need to zero fill here, not clobber
-	 * region, as we want to ensure that valgrind thinks
-	 * all of the outgoing buffer has been written to
-	 * so a send() or write() won't trap an error.
-	 * JRA.
-	 */
-	memset(dest, '\0', dest_len);
-#endif
 
 	if (!(flags & STR_ASCII) && \
 	    (flags & STR_UNICODE)) {
@@ -1595,20 +1557,14 @@ ssize_t push_string(void *dest, const char *src, size_t dest_len, int flags)
  The resulting string in "dest" is always null terminated.
 **/
 
-size_t pull_string_fn(const char *function,
-			unsigned int line,
-			const void *base_ptr,
-			uint16 smb_flags2,
-			char *dest,
-			const void *src,
-			size_t dest_len,
-			size_t src_len,
-			int flags)
+size_t pull_string_fn(const void *base_ptr,
+		   uint16 smb_flags2,
+		   char *dest,
+		   const void *src,
+		   size_t dest_len,
+		   size_t src_len,
+		   int flags)
 {
-#ifdef DEVELOPER
-	clobber_region(function, line, dest, dest_len);
-#endif
-
 	if ((base_ptr == NULL) && ((flags & (STR_ASCII|STR_UNICODE)) == 0)) {
 		smb_panic("No base ptr to get flg2 and neither ASCII nor "
 			  "UNICODE defined");
@@ -1637,15 +1593,13 @@ size_t pull_string_fn(const char *function,
  The resulting string in "dest" is always null terminated.
 **/
 
-size_t pull_string_talloc_fn(const char *function,
-			unsigned int line,
-			TALLOC_CTX *ctx,
-			const void *base_ptr,
-			uint16 smb_flags2,
-			char **ppdest,
-			const void *src,
-			size_t src_len,
-			int flags)
+size_t pull_string_talloc(TALLOC_CTX *ctx,
+			  const void *base_ptr,
+			  uint16 smb_flags2,
+			  char **ppdest,
+			  const void *src,
+			  size_t src_len,
+			  int flags)
 {
 	if ((base_ptr == NULL) && ((flags & (STR_ASCII|STR_UNICODE)) == 0)) {
 		smb_panic("No base ptr to get flg2 and neither ASCII nor "
diff --git a/source3/lib/clobber.c b/source3/lib/clobber.c
deleted file mode 100644
index e77e786fb5..0000000000
--- a/source3/lib/clobber.c
+++ /dev/null
@@ -1,63 +0,0 @@
-/* 
-   Unix SMB/CIFS implementation.
-   Samba utility functions
-   Copyright (C) Martin Pool     2003
-   Copyright (C) Andrew Bartlett 2003
-   
-   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 "includes.h"
-
-#ifdef DEVELOPER
-const char *global_clobber_region_function;
-unsigned int global_clobber_region_line;
-#endif
-
-/**
- * In developer builds, clobber a region of memory.
- *
- * If we think a string buffer is longer than it really is, this ought
- * to make the failure obvious, by segfaulting (if in the heap) or by
- * killing the return address (on the stack), or by trapping under a
- * memory debugger.
- *
- * This is meant to catch possible string overflows, even if the
- * actual string copied is not big enough to cause an overflow.
- *
- * In addition, under Valgrind the buffer is marked as uninitialized.
- **/
-void clobber_region(const char *fn, unsigned int line, char *dest, size_t len)
-{
-#ifdef DEVELOPER
-	global_clobber_region_function = fn;
-	global_clobber_region_line = line;
-
-	/* F1 is odd and 0xf1f1f1f1 shouldn't be a valid pointer */
-	memset(dest, 0xF1, len);
-#ifdef VALGRIND
-	/* Even though we just wrote to this, from the application's
-	 * point of view it is not initialized.
-	 *
-	 * (This is not redundant with the clobbering above.  The
-	 * marking might not actually take effect if we're not running
-	 * under valgrind.) */
-#if defined(VALGRIND_MAKE_MEM_UNDEFINED)
-	VALGRIND_MAKE_MEM_UNDEFINED(dest, len);
-#elif defined(VALGRIND_MAKE_WRITABLE)
-	VALGRIND_MAKE_WRITABLE(dest, len);
-#endif
-#endif /* VALGRIND */
-#endif /* DEVELOPER */
-}
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 79b10fda5e..b99d9d42a8 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -26,9 +26,6 @@
 #include "secrets.h"
 #include "ctdbd_conn.h"
 
-extern char *global_clobber_region_function;
-extern unsigned int global_clobber_region_line;
-
 /* Max allowable allococation - 256mb - 0x10000000 */
 #define MAX_ALLOC_SIZE (1024*1024*256)
 
@@ -1349,17 +1346,6 @@ void smb_panic_s3(const char *why)
 	char *cmd;
 	int result;
 
-#ifdef DEVELOPER
-	{
-
-		if (global_clobber_region_function) {
-			DEBUG(0,("smb_panic: clobber_region() last called from [%s(%u)]\n",
-					 global_clobber_region_function,
-					 global_clobber_region_line));
-		} 
-	}
-#endif
-
 	DEBUG(0,("PANIC (pid %llu): %s\n",
 		    (unsigned long long)sys_getpid(), why));
 	log_stack_trace();
diff --git a/source3/lib/util_sock.c b/source3/lib/util_sock.c
index 3c97495099..eea153ad91 100644
--- a/source3/lib/util_sock.c
+++ b/source3/lib/util_sock.c
@@ -1380,10 +1380,6 @@ const char *get_peer_name(int fd, bool force_lookup)
 		}
 	}
 
-	/* can't pass the same source and dest strings in when you
-	   use --enable-developer or the clobber_region() call will
-	   get you */
-
 	strlcpy(tmp_name, name_buf, sizeof(tmp_name));
 	alpha_strcpy(name_buf, tmp_name, "_-.", sizeof(name_buf));
 	if (strstr(name_buf,"..")) {
diff --git a/source3/lib/util_str.c b/source3/lib/util_str.c
index 6a17297231..a1dfc70707 100644
--- a/source3/lib/util_str.c
+++ b/source3/lib/util_str.c
@@ -500,24 +500,16 @@ bool strhaslower(const char *s)
  include the terminating zero.
 **/
 
-char *safe_strcpy_fn(const char *fn,
-		int line,
-		char *dest,
-		const char *src,
-		size_t maxlength)
+char *safe_strcpy_fn(char *dest,
+		     const char *src,
+		     size_t maxlength)
 {
 	size_t len;
 
 	if (!dest) {
-		DEBUG(0,("ERROR: NULL dest in safe_strcpy, "
-			"called from [%s][%d]\n", fn, line));
-		return NULL;
+		smb_panic("ERROR: NULL dest in safe_strcpy");
 	}
 
-#ifdef DEVELOPER
-	clobber_region(fn,line,dest, maxlength+1);
-#endif
-
 	if (!src) {
 		*dest = 0;
 		return dest;
@@ -542,18 +534,14 @@ char *safe_strcpy_fn(const char *fn,
  Safe string cat into a string. maxlength does not
  include the terminating zero.
 **/
-char *safe_strcat_fn(const char *fn,
-		int line,
-		char *dest,
-		const char *src,
-		size_t maxlength)
+char *safe_strcat_fn(char *dest,
+		     const char *src,
+		     size_t maxlength)
 {
 	size_t src_len, dest_len;
 
 	if (!dest) {
-		DEBUG(0,("ERROR: NULL dest in safe_strcat, "
-			"called from [%s][%d]\n", fn, line));
-		return NULL;
+		smb_panic("ERROR: NULL dest in safe_strcat");
 	}
 
 	if (!src)
@@ -562,10 +550,6 @@ char *safe_strcat_fn(const char *fn,
 	src_len = strnlen(src, maxlength + 1);
 	dest_len = strnlen(dest, maxlength + 1);
 
-#ifdef DEVELOPER
-	clobber_region(fn, line, dest + dest_len, maxlength + 1 - dest_len);
-#endif
-
 	if (src_len + dest_len > maxlength) {
 		DEBUG(0,("ERROR: string overflow by %d "
 			"in safe_strcat [%.50s]\n",
@@ -589,23 +573,15 @@ char *safe_strcat_fn(const char *fn,
  characters. Don't change it !
 **/
 
-char *alpha_strcpy_fn(const char *fn,
-		int line,
-		char *dest,
-		const char *src,
-		const char *other_safe_chars,
-		size_t maxlength)
+char *alpha_strcpy_fn(char *dest,
+		      const char *src,
+		      const char *other_safe_chars,
+		      size_t maxlength)
 {
 	size_t len, i;
 
-#ifdef DEVELOPER
-	clobber_region(fn, line, dest, maxlength);
-#endif
-
 	if (!dest) {
-		DEBUG(0,("ERROR: NULL dest in alpha_strcpy, "
-			"called from [%s][%d]\n", fn, line));
-		return NULL;
+		smb_panic("ERROR: NULL dest in alpha_strcpy");
 	}
 
 	if (!src) {
@@ -638,18 +614,12 @@ char *alpha_strcpy_fn(const char *fn,
  Like strncpy but always null terminates. Make sure there is room!
  The variable n should always be one less than the available size.
 **/
-char *StrnCpy_fn(const char *fn, int line,char *dest,const char *src,size_t n)
+char *StrnCpy_fn(char *dest,const char *src,size_t n)
 {
 	char *d = dest;
 
-#ifdef DEVELOPER
-	clobber_region(fn, line, dest, n+1);
-#endif
-
 	if (!dest) {
-		DEBUG(0,("ERROR: NULL dest in StrnCpy, "
-			"called from [%s][%d]\n", fn, line));
-		return(NULL);
+		smb_panic("ERROR: NULL dest in StrnCpy");
 	}
 
 	if (!src) {
@@ -677,9 +647,6 @@ static char *strncpyn(char *dest, const char *src, size_t n, char c)
 	char *p;
 	size_t str_len;
 
-#ifdef DEVELOPER
-	clobber_region(dest, n+1);
-#endif
 	p = strchr_m(src, c);
 	if (p == NULL) {
 		DEBUG(5, ("strncpyn: separator character (%c) not found\n", c));