[PATCHSET] Clean up debug.c

Volker Lendecke Volker.Lendecke at SerNet.DE
Fri Aug 1 04:02:48 MDT 2014


On Thu, Jul 31, 2014 at 05:57:07PM +0200, Michael Adam wrote:
> On 2014-07-31 at 08:36 +0200, Michael Adam wrote:
> > FYI: I am looking at these.
> 
> nice! ... pushed

Thanks!

Attached find a small follow-up patchset that makes DEBUG a
small subsystem that has only very few dependencies.

Review would be 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 cbc3ec48f2a0b04e7d0340b994465b4edd6b60a9 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Jul 2014 20:35:10 +0200
Subject: [PATCH 1/5] lib: Add close_low_fd

This factors out the essential code from close_low_fds for one file
descriptor: Redirect a fd to /dev/null

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/util/become_daemon.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 lib/util/samba_util.h    |  5 +++++
 2 files changed, 48 insertions(+)

diff --git a/lib/util/become_daemon.c b/lib/util/become_daemon.c
index 0671a1e..eba0cae 100644
--- a/lib/util/become_daemon.c
+++ b/lib/util/become_daemon.c
@@ -32,6 +32,49 @@
  Close the low 3 fd's and open dev/null in their place.
 ********************************************************************/
 
+_PUBLIC_ int close_low_fd(int fd)
+{
+#ifndef VALGRIND
+	int ret, dev_null;
+
+	dev_null = open("/dev/null", O_RDWR, 0);
+
+	if ((dev_null == -1) && (errno = ENFILE)) {
+		/*
+		 * Try to free up an fd
+		 */
+		ret = close(fd);
+		if (ret != 0) {
+			return errno;
+		}
+	}
+
+	dev_null = open("/dev/null", O_RDWR, 0);
+	if (dev_null == -1) {
+		dev_null = open("/dev/null", O_WRONLY, 0);
+	}
+	if (dev_null == -1) {
+		return errno;
+	}
+
+	if (dev_null == fd) {
+		/*
+		 * This can happen in the ENFILE case above
+		 */
+		return 0;
+	}
+
+	ret = dup2(dev_null, fd);
+	if (ret == -1) {
+		int err = errno;
+		close(dev_null);
+		return err;
+	}
+	close(dev_null);
+#endif
+	return 0;
+}
+
 _PUBLIC_ void close_low_fds(bool stdin_too, bool stdout_too, bool stderr_too)
 {
 #ifndef VALGRIND
diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index 2ffe028..b31ee1f 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -839,6 +839,11 @@ _PUBLIC_ int idr_remove(struct idr_context *idp, int id);
 /* The following definitions come from lib/util/become_daemon.c  */
 
 /**
+ Close a fd and open dev/null in its place
+**/
+_PUBLIC_ int close_low_fd(int fd);
+
+/**
  Close the low 3 fd's and open dev/null in their place
 **/
 _PUBLIC_ void close_low_fds(bool stdin_too, bool stdout_too, bool stderr_too);
-- 
1.8.1.2


From 55e42d30bd0073db69a74ddc2b4eab9424b614a3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Jul 2014 20:42:18 +0200
Subject: [PATCH 2/5] lib: Use close_low_fd in close_low_fds

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/util/become_daemon.c | 52 ++++++++++++++++++------------------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/lib/util/become_daemon.c b/lib/util/become_daemon.c
index eba0cae..d940cd7 100644
--- a/lib/util/become_daemon.c
+++ b/lib/util/become_daemon.c
@@ -77,42 +77,28 @@ _PUBLIC_ int close_low_fd(int fd)
 
 _PUBLIC_ void close_low_fds(bool stdin_too, bool stdout_too, bool stderr_too)
 {
-#ifndef VALGRIND
-	int fd;
-	int i;
-
-	if (stdin_too)
-		close(0);
-	if (stdout_too)
-		close(1);
-
-	if (stderr_too)
-		close(2);
-
-	/* try and use up these file descriptors, so silly
-		library routines writing to stdout etc won't cause havoc */
-	for (i=0;i<3;i++) {
-		if (i == 0 && !stdin_too)
-			continue;
-		if (i == 1 && !stdout_too)
-			continue;
-		if (i == 2 && !stderr_too)
-			continue;
-
-		fd = open("/dev/null",O_RDWR,0);
-		if (fd < 0)
-			fd = open("/dev/null",O_WRONLY,0);
-		if (fd < 0) {
-			DEBUG(0,("Can't open /dev/null\n"));
-			return;
+
+	if (stdin_too) {
+		int ret = close_low_fd(0);
+		if (ret != 0) {
+			DEBUG(0, ("%s: close_low_fd(0) failed: %s\n",
+				  __func__, strerror(ret)));
 		}
-		if (fd != i) {
-			DEBUG(0,("Didn't get file descriptor %d\n",i));
-			close(fd);
-			return;
+	}
+	if (stdout_too) {
+		int ret = close_low_fd(1);
+		if (ret != 0) {
+			DEBUG(0, ("%s: close_low_fd(1) failed: %s\n",
+				  __func__, strerror(ret)));
+		}
+	}
+	if (stderr_too) {
+		int ret = close_low_fd(2);
+		if (ret != 0) {
+			DEBUG(0, ("%s: close_low_fd(2) failed: %s\n",
+				  __func__, strerror(ret)));
 		}
 	}
-#endif
 }
 
 /****************************************************************************
-- 
1.8.1.2


From 83a00568d0f7b8d7e821c80dea3b7078c46dee07 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 29 Jul 2014 20:43:05 +0200
Subject: [PATCH 3/5] debug: Use close_low_fd in reopen_logs_internal

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/util/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/util/debug.c b/lib/util/debug.c
index 33cbed2..f83b14c 100644
--- a/lib/util/debug.c
+++ b/lib/util/debug.c
@@ -617,7 +617,7 @@ bool reopen_logs_internal(void)
 			   at the logfile.  There really isn't much
 			   that can be done on such a fundamental
 			   failure... */
-			close_low_fds(false, false, true);
+			close_low_fd(2);
 		}
 	}
 
-- 
1.8.1.2


From b133a5a3828f3876d968325881394f7c19e35bf4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 31 Jul 2014 09:40:04 +0000
Subject: [PATCH 4/5] lib: Make close_low_fd() independently linkable

---
 lib/util/become_daemon.c | 44 +-------------------------------
 lib/util/close_low_fd.c  | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/util/close_low_fd.h  | 28 +++++++++++++++++++++
 lib/util/debug.c         |  1 +
 lib/util/samba_util.h    |  7 ------
 lib/util/wscript_build   |  7 +++++-
 6 files changed, 101 insertions(+), 51 deletions(-)
 create mode 100644 lib/util/close_low_fd.c
 create mode 100644 lib/util/close_low_fd.h

diff --git a/lib/util/become_daemon.c b/lib/util/become_daemon.c
index d940cd7..17e0baf 100644
--- a/lib/util/become_daemon.c
+++ b/lib/util/become_daemon.c
@@ -27,54 +27,12 @@
 #if HAVE_SYSTEMD
 #include <systemd/sd-daemon.h>
 #endif
+#include "lib/util/close_low_fd.h"
 
 /*******************************************************************
  Close the low 3 fd's and open dev/null in their place.
 ********************************************************************/
 
-_PUBLIC_ int close_low_fd(int fd)
-{
-#ifndef VALGRIND
-	int ret, dev_null;
-
-	dev_null = open("/dev/null", O_RDWR, 0);
-
-	if ((dev_null == -1) && (errno = ENFILE)) {
-		/*
-		 * Try to free up an fd
-		 */
-		ret = close(fd);
-		if (ret != 0) {
-			return errno;
-		}
-	}
-
-	dev_null = open("/dev/null", O_RDWR, 0);
-	if (dev_null == -1) {
-		dev_null = open("/dev/null", O_WRONLY, 0);
-	}
-	if (dev_null == -1) {
-		return errno;
-	}
-
-	if (dev_null == fd) {
-		/*
-		 * This can happen in the ENFILE case above
-		 */
-		return 0;
-	}
-
-	ret = dup2(dev_null, fd);
-	if (ret == -1) {
-		int err = errno;
-		close(dev_null);
-		return err;
-	}
-	close(dev_null);
-#endif
-	return 0;
-}
-
 _PUBLIC_ void close_low_fds(bool stdin_too, bool stdout_too, bool stderr_too)
 {
 
diff --git a/lib/util/close_low_fd.c b/lib/util/close_low_fd.c
new file mode 100644
index 0000000..b11d25f
--- /dev/null
+++ b/lib/util/close_low_fd.c
@@ -0,0 +1,65 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Samba utility functions
+ * Copyright (C) Volker Lendecke 2014
+ *
+ * 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 "system/filesys.h"
+#include "close_low_fd.h"
+
+_PUBLIC_ int close_low_fd(int fd)
+{
+#ifndef VALGRIND
+	int ret, dev_null;
+
+	dev_null = open("/dev/null", O_RDWR, 0);
+
+	if ((dev_null == -1) && (errno = ENFILE)) {
+		/*
+		 * Try to free up an fd
+		 */
+		ret = close(fd);
+		if (ret != 0) {
+			return errno;
+		}
+	}
+
+	dev_null = open("/dev/null", O_RDWR, 0);
+	if (dev_null == -1) {
+		dev_null = open("/dev/null", O_WRONLY, 0);
+	}
+	if (dev_null == -1) {
+		return errno;
+	}
+
+	if (dev_null == fd) {
+		/*
+		 * This can happen in the ENFILE case above
+		 */
+		return 0;
+	}
+
+	ret = dup2(dev_null, fd);
+	if (ret == -1) {
+		int err = errno;
+		close(dev_null);
+		return err;
+	}
+	close(dev_null);
+#endif
+	return 0;
+}
diff --git a/lib/util/close_low_fd.h b/lib/util/close_low_fd.h
new file mode 100644
index 0000000..954d1d2
--- /dev/null
+++ b/lib/util/close_low_fd.h
@@ -0,0 +1,28 @@
+/*
+ * Unix SMB/CIFS implementation.
+ * Samba utility functions
+ * Copyright (C) Volker Lendecke 2014
+ *
+ * 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 _CLOSE_LOW_FD_H
+#define _CLOSE_LOW_FD_H
+
+/*
+ * Redirect "fd" to /dev/null
+ */
+int close_low_fd(int fd);
+
+#endif
diff --git a/lib/util/debug.c b/lib/util/debug.c
index f83b14c..2779dd3 100644
--- a/lib/util/debug.c
+++ b/lib/util/debug.c
@@ -23,6 +23,7 @@
 #include "system/filesys.h"
 #include "system/syslog.h"
 #include "lib/util/time_basic.h"
+#include "lib/util/close_low_fd.h"
 
 /* define what facility to use for syslog */
 #ifndef SYSLOG_FACILITY
diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index b31ee1f..233b5fd 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -836,13 +836,6 @@ _PUBLIC_ void *idr_find(struct idr_context *idp, int id);
 */
 _PUBLIC_ int idr_remove(struct idr_context *idp, int id);
 
-/* The following definitions come from lib/util/become_daemon.c  */
-
-/**
- Close a fd and open dev/null in its place
-**/
-_PUBLIC_ int close_low_fd(int fd);
-
 /**
  Close the low 3 fd's and open dev/null in their place
 **/
diff --git a/lib/util/wscript_build b/lib/util/wscript_build
index 8be2c2e..6f625dc 100755
--- a/lib/util/wscript_build
+++ b/lib/util/wscript_build
@@ -5,6 +5,11 @@ bld.SAMBA_SUBSYSTEM('time-basic',
                     deps='replace',
                     local_include=False)
 
+bld.SAMBA_SUBSYSTEM('close-low-fd',
+                    source='close_low_fd.c',
+                    deps='replace',
+                    local_include=False)
+
 bld.SAMBA_LIBRARY('samba-util',
                   source='''talloc_stack.c smb_threads.c xfile.c data_blob.c
                     util_file.c time.c rbtree.c rfc1738.c select.c getpass.c
@@ -14,7 +19,7 @@ bld.SAMBA_LIBRARY('samba-util',
                     util_str.c util_str_common.c substitute.c ms_fnmatch.c
                     server_id.c dprintf.c parmlist.c bitmap.c pidfile.c
                     tevent_debug.c util_process.c memcache.c''',
-                  deps='DYNCONFIG time-basic',
+                  deps='DYNCONFIG time-basic close-low-fd',
                   public_deps='talloc tevent execinfo pthread LIBCRYPTO charset util_setid systemd-daemon',
                   public_headers='debug.h attr.h byteorder.h data_blob.h memory.h safe_string.h time.h talloc_stack.h xfile.h dlinklist.h samba_util.h string_wrappers.h',
                   header_path= [ ('dlinklist.h samba_util.h', '.'), ('*', 'util') ],
-- 
1.8.1.2


From 21904b8185708b8261f955c2066c65332aedb08c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Jul 2014 14:12:54 +0000
Subject: [PATCH 5/5] lib: Make DEBUG a subsystem of its own

In the future this might become a library, but even with the SUBSYSTEM
it should be clear what debug.c depends upon.

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

diff --git a/lib/util/wscript_build b/lib/util/wscript_build
index 6f625dc..7c4abf9 100755
--- a/lib/util/wscript_build
+++ b/lib/util/wscript_build
@@ -10,16 +10,21 @@ bld.SAMBA_SUBSYSTEM('close-low-fd',
                     deps='replace',
                     local_include=False)
 
+bld.SAMBA_SUBSYSTEM('samba-debug',
+                    source='debug.c',
+                    deps='replace time-basic close-low-fd talloc',
+                    local_include=False)
+
 bld.SAMBA_LIBRARY('samba-util',
                   source='''talloc_stack.c smb_threads.c xfile.c data_blob.c
                     util_file.c time.c rbtree.c rfc1738.c select.c getpass.c
                     genrand.c fsusage.c blocking.c become_daemon.c
                     signal.c system.c params.c util.c util_id.c util_net.c
-                    util_strlist.c util_paths.c idtree.c debug.c fault.c base64.c
+                    util_strlist.c util_paths.c idtree.c fault.c base64.c
                     util_str.c util_str_common.c substitute.c ms_fnmatch.c
                     server_id.c dprintf.c parmlist.c bitmap.c pidfile.c
                     tevent_debug.c util_process.c memcache.c''',
-                  deps='DYNCONFIG time-basic close-low-fd',
+                  deps='DYNCONFIG time-basic close-low-fd samba-debug',
                   public_deps='talloc tevent execinfo pthread LIBCRYPTO charset util_setid systemd-daemon',
                   public_headers='debug.h attr.h byteorder.h data_blob.h memory.h safe_string.h time.h talloc_stack.h xfile.h dlinklist.h samba_util.h string_wrappers.h',
                   header_path= [ ('dlinklist.h samba_util.h', '.'), ('*', 'util') ],
-- 
1.8.1.2



More information about the samba-technical mailing list