From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 18 Oct 2022 12:44:01 +0000 (-0400)
Subject: cleanups and fixes for #4777
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bd8032f2bd486c8366d4b99283086e2607130fc1;p=thirdparty%2Ffreeradius-server.git

cleanups and fixes for #4777

Move common macros and includes to a common header file.

Create a macro which wraps OpenBSD's closefrom() in a macro,
so that the source code isn't littered with ifdef's
---

diff --git a/src/lib/server/exec.c b/src/lib/server/exec.c
index 3bc14f5ba0c..bd750e5b0a0 100644
--- a/src/lib/server/exec.c
+++ b/src/lib/server/exec.c
@@ -24,41 +24,10 @@
  */
 RCSID("$Id$")
 
-#include <freeradius-devel/server/exec.h>
 #include <freeradius-devel/util/debug.h>
 #include <freeradius-devel/server/request.h>
 #include <freeradius-devel/server/util.h>
-#include <freeradius-devel/unlang/interpret.h>
-
-#include <freeradius-devel/util/dlist.h>
-#include <freeradius-devel/util/misc.h>
-#include <freeradius-devel/util/pair_legacy.h>
-#include <freeradius-devel/util/syserror.h>
-#include <freeradius-devel/util/atexit.h>
-
-#include <freeradius-devel/protocol/freeradius/freeradius.internal.h>
-
-#include <sys/file.h>
-
-#include <fcntl.h>
-#include <ctype.h>
-#include <signal.h>
-
-#if defined(__APPLE__) || defined(__FreeBSD__)
-extern char **environ;
-#else
-#  include <unistd.h>
-#endif
-
-#ifdef HAVE_SYS_WAIT_H
-#  include <sys/wait.h>
-#endif
-#ifndef WEXITSTATUS
-#  define WEXITSTATUS(stat_val) ((unsigned)(stat_val) >> 8)
-#endif
-#ifndef WIFEXITED
-#  define WIFEXITED(stat_val) (((stat_val) & 0x7f) == 0)
-#endif
+#include <freeradius-devel/server/exec_priv.h>
 
 #define MAX_ENVP 1024
 
@@ -309,7 +278,7 @@ static NEVER_RETURNS void exec_child(request_t *request, char **argv, char **env
 	 *	want to leave dangling FD's for the child process
 	 *	to play funky games with, so we close them.
 	 */
-	closefrom(STDERR_FILENO + 1);
+	fr_closefrom(STDERR_FILENO + 1);
 
 	/*
 	 *	Disarm the thread local destructors
diff --git a/src/lib/server/exec.h b/src/lib/server/exec.h
index 7d7f27818f0..7d0af0f6242 100644
--- a/src/lib/server/exec.h
+++ b/src/lib/server/exec.h
@@ -18,8 +18,8 @@
 /**
  * $Id$
  *
- * @file lib/server/exfile.h
- * @brief API for managing concurrent file access.
+ * @file lib/server/exec.h
+ * @brief Asynchronous exec
  *
  * @copyright 2014 The FreeRADIUS server project
  */
@@ -29,8 +29,6 @@ RCSIDH(exec_h, "$Id$")
 extern "C" {
 #endif
 
-#include <unistd.h>
-
 #define EXEC_TIMEOUT		10	//!< Default wait time for exec calls (in seconds).
 
 #ifdef __cplusplus
diff --git a/src/lib/server/exec_legacy.c b/src/lib/server/exec_legacy.c
index 5df3ac2d3dd..ffe95dbba18 100644
--- a/src/lib/server/exec_legacy.c
+++ b/src/lib/server/exec_legacy.c
@@ -24,36 +24,11 @@
  */
 RCSID("$Id$")
 
-#include <freeradius-devel/server/exec.h>
-#include <freeradius-devel/server/exec_legacy.h>
 #include <freeradius-devel/util/debug.h>
 #include <freeradius-devel/server/request.h>
 #include <freeradius-devel/server/util.h>
-#include <freeradius-devel/unlang/interpret.h>
-
-#include <freeradius-devel/util/dlist.h>
-#include <freeradius-devel/util/misc.h>
-#include <freeradius-devel/util/pair_legacy.h>
-#include <freeradius-devel/util/syserror.h>
-#include <freeradius-devel/util/atexit.h>
-
-#include <freeradius-devel/protocol/freeradius/freeradius.internal.h>
-
-#include <sys/file.h>
-
-#include <fcntl.h>
-#include <ctype.h>
-#include <signal.h>
-
-#ifdef HAVE_SYS_WAIT_H
-#  include <sys/wait.h>
-#endif
-#ifndef WEXITSTATUS
-#  define WEXITSTATUS(stat_val) ((unsigned)(stat_val) >> 8)
-#endif
-#ifndef WIFEXITED
-#  define WIFEXITED(stat_val) (((stat_val) & 0x7f) == 0)
-#endif
+#include <freeradius-devel/server/exec_legacy.h>
+#include <freeradius-devel/server/exec_priv.h>
 
 #define MAX_ARGV (256)
 
@@ -196,7 +171,7 @@ static NEVER_RETURNS void exec_child_legacy(request_t *request, char **argv, cha
 	 *	want to leave dangling FD's for the child process
 	 *	to play funky games with, so we close them.
 	 */
-	closefrom(STDERR_FILENO + 1);
+	os_closefrom(STDERR_FILENO + 1);
 
 	/*
 	 *	Disarm the thread local destructors
diff --git a/src/lib/server/exec_legacy.h b/src/lib/server/exec_legacy.h
index 57792662342..7aa6b7890c2 100644
--- a/src/lib/server/exec_legacy.h
+++ b/src/lib/server/exec_legacy.h
@@ -29,8 +29,6 @@ RCSIDH(exec_legacy_h, "$Id$")
 extern "C" {
 #endif
 
-#include <unistd.h>
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/lib/server/exec_priv.h b/src/lib/server/exec_priv.h
new file mode 100644
index 00000000000..f80deb6c26d
--- /dev/null
+++ b/src/lib/server/exec_priv.h
@@ -0,0 +1,86 @@
+#pragma once
+/*
+ *  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 2 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, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+/**
+ * $Id$
+ *
+ * @file lib/server/exec_priv.h
+ * @brief Private exec APIs
+ *
+ * @copyright 2014 The FreeRADIUS server project
+ */
+RCSIDH(exec_priv_h, "$Id$")
+
+#include <freeradius-devel/server/exec.h>
+
+#include <freeradius-devel/unlang/interpret.h>
+
+#include <freeradius-devel/util/dlist.h>
+#include <freeradius-devel/util/misc.h>
+#include <freeradius-devel/util/pair_legacy.h>
+#include <freeradius-devel/util/syserror.h>
+#include <freeradius-devel/util/atexit.h>
+
+#include <sys/types.h>
+
+#include <freeradius-devel/protocol/freeradius/freeradius.internal.h>
+
+#include <sys/file.h>
+
+#include <fcntl.h>
+#include <ctype.h>
+#include <signal.h>
+
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#if defined(__APPLE__) || defined(__FreeBSD__)
+extern char **environ;
+#else
+#  include <unistd.h>
+#endif
+
+#ifdef HAVE_SYS_WAIT_H
+#  include <sys/wait.h>
+#endif
+#ifndef WEXITSTATUS
+#  define WEXITSTATUS(stat_val) ((unsigned)(stat_val) >> 8)
+#endif
+#ifndef WIFEXITED
+#  define WIFEXITED(stat_val) (((stat_val) & 0x7f) == 0)
+#endif
+
+#if defined(OpenBSD)
+/*
+ *	The standard closefrom() returns void.
+ *	OpenBSD's closefrom ()returns int and can be EINTR'd.
+ *	So we have to keep calling it until it no longer returns EINTR
+ */
+#define fr_closefrom(_x) do {		\
+		errno = 0;		\
+		closefrom(_x);		\
+	} while (errno == EINTR)	\
+
+#else
+#define fr_closefrom closefrom
+#endif
+
+#ifdef __cplusplus
+}
+#endif