[icinga-checkins] icinga.org: icinga2/master: Bugfixes for the Process class.

git at icinga.org git at icinga.org
Sun Feb 10 12:45:49 CET 2013


Module: icinga2
Branch: master
Commit: 0e7a3c67ec3fed142cb5d5d915196ab6591d6b9a
URL:    https://git.icinga.org/?p=icinga2.git;a=commit;h=0e7a3c67ec3fed142cb5d5d915196ab6591d6b9a

Author: Gunnar Beutner <gunnar.beutner at netways.de>
Date:   Sun Feb 10 12:45:39 2013 +0100

Bugfixes for the Process class.

---

 lib/base/process.cpp                  |  160 +++++++++++++++------------------
 lib/base/process.h                    |   13 +--
 lib/icinga/pluginchecktask.cpp        |    2 +-
 lib/icinga/pluginnotificationtask.cpp |    2 +-
 lib/icinga/service-notification.cpp   |    2 +-
 5 files changed, 78 insertions(+), 101 deletions(-)

diff --git a/lib/base/process.cpp b/lib/base/process.cpp
index d57ae7b..0d1d70e 100644
--- a/lib/base/process.cpp
+++ b/lib/base/process.cpp
@@ -26,9 +26,10 @@ boost::mutex Process::m_Mutex;
 deque<Process::Ptr> Process::m_Tasks;
 int Process::m_TaskFd;
 
-Process::Process(const String& command, const Dictionary::Ptr& environment)
-	: AsyncTask<Process, ProcessResult>(), m_Command(command),
-	  m_Environment(environment), m_FP(NULL)
+extern char **environ;
+
+Process::Process(const vector<String>& arguments, const Dictionary::Ptr& extraEnvironment)
+	: AsyncTask<Process, ProcessResult>(), m_FP(NULL)
 {
 	assert(Application::IsMainThread());
 
@@ -45,6 +46,49 @@ Process::Process(const String& command, const Dictionary::Ptr& environment)
 
 		m_ThreadCreated = true;
 	}
+
+	// build argv
+	m_Arguments = new char *[arguments.size() + 1];
+
+	for (int i = 0; i < arguments.size(); i++)
+		m_Arguments[i] = strdup(arguments[i].CStr());
+
+	m_Arguments[arguments.size()] = NULL;
+
+	// build envp
+	int envc = 0;
+
+	/* count existing environment variables */
+	while (environ[envc] != NULL)
+		envc++;
+
+	m_Environment = new char *[envc + (extraEnvironment ? extraEnvironment->GetLength() : 0) + 1];
+
+	for (int i = 0; i < envc; i++)
+		m_Environment[i] = strdup(environ[i]);
+
+	if (extraEnvironment) {
+		String key;
+		Value value;
+		int index = envc;
+		BOOST_FOREACH(tie(key, value), extraEnvironment) {
+			String kv = key + "=" + Convert::ToString(value);
+			m_Environment[index] = strdup(kv.CStr());
+			index++;
+		}
+	}
+
+	m_Environment[envc + (extraEnvironment ? extraEnvironment->GetLength() : 0)] = NULL;
+}
+
+vector<String> Process::ParseCommand(const String& command)
+{
+	// TODO: implement
+	vector<String> args;
+	args.push_back("sh");
+	args.push_back("-c");
+	args.push_back(command);
+	return args;
 }
 
 void Process::Run(void)
@@ -117,7 +161,7 @@ void Process::WorkerThreadProc(int taskFd)
 				try {
 					task->InitTask();
 
-					int fd = task->GetFD();
+					int fd = fileno(task->m_FP);
 					if (fd >= 0)
 						tasks[fd] = task;
 				} catch (...) {
@@ -150,18 +194,10 @@ void Process::WorkerThreadProc(int taskFd)
 	}
 }
 
-void Process::Spawn(const String& command, const Dictionary::Ptr& env)
+void Process::InitTask(void)
 {
-	vector<String> args;
-	args.push_back("sh");
-	args.push_back("-c");
-	args.push_back(command);
-
-	return Spawn(args, env);
-}
+	m_Result.ExecutionStart = Utility::GetTime();
 
-void Process::Spawn(const vector<String>& args, const Dictionary::Ptr& extraEnv)
-{
 	assert(m_FP == NULL);
 
 #ifdef _MSC_VER
@@ -172,37 +208,6 @@ void Process::Spawn(const vector<String>& args, const Dictionary::Ptr& extraEnv)
 	if (pipe(fds) < 0)
 		BOOST_THROW_EXCEPTION(PosixException("pipe() failed.", errno));
 
-	char **argv = new char *[args.size() + 1];
-
-	for (int i = 0; i < args.size(); i++)
-		argv[i] = strdup(args[i].CStr());
-
-	argv[args.size()] = NULL;
-
-	int envc = 0;
-
-	/* count existing environment variables */
-	while (environ[envc] != NULL)
-		envc++;
-
-	char **envp = new char *[envc + (extraEnv ? extraEnv->GetLength() : 0) + 1];
-
-	for (int i = 0; i < envc; i++)
-		envp[i] = environ[i];
-
-	if (extraEnv) {
-		String key;
-		Value value;
-		int index = envc;
-		BOOST_FOREACH(tie(key, value), extraEnv) {
-			String kv = key + "=" + Convert::ToString(value);
-			envp[index] = strdup(kv.CStr());
-			index++;
-		}
-	}
-
-	envp[envc + (extraEnv ? extraEnv->GetLength() : 0)] = NULL;
-
 #ifdef HAVE_VFORK
 	m_Pid = vfork();
 #else /* HAVE_VFORK */
@@ -213,7 +218,8 @@ void Process::Spawn(const vector<String>& args, const Dictionary::Ptr& extraEnv)
 		BOOST_THROW_EXCEPTION(PosixException("fork() failed.", errno));
 
 	if (m_Pid == 0) {
-		/* child */
+		// child process
+
 		if (dup2(fds[1], STDOUT_FILENO) < 0 || dup2(fds[1], STDERR_FILENO) < 0) {
 			perror("dup2() failed.");
 			_exit(128);
@@ -221,26 +227,27 @@ void Process::Spawn(const vector<String>& args, const Dictionary::Ptr& extraEnv)
 
 		(void) close(fds[1]);
 
-		if (execvpe(argv[0], argv, envp) < 0) {
+		if (execvpe(m_Arguments[0], m_Arguments, m_Environment) < 0) {
 			perror("execvpe() failed.");
 			_exit(128);
 		}
 
 		_exit(128);
 	} else {
-		for (int i = 0; i < args.size(); i++)
-			free(argv[i]);
+		// parent process
 
-		delete [] argv;
+		// free arguments
+		for (int i = 0; m_Arguments[i] != NULL; i++)
+			free(m_Arguments[i]);
 
-		if (extraEnv) {
-			for (int i = envc; i < envc + extraEnv->GetLength(); i++)
-				free(envp[i]);
-		}
+		delete [] m_Arguments;
+
+		// free environment
+		for (int i = 0; m_Environment[i] != NULL; i++)
+			free(m_Environment[i]);
 
-		delete [] envp;
+		delete [] m_Environment;
 
-		/* parent */
 		(void) close(fds[1]);
 
 		m_FP = fdopen(fds[0], "r");
@@ -251,28 +258,6 @@ void Process::Spawn(const vector<String>& args, const Dictionary::Ptr& extraEnv)
 #endif /* _MSC_VER */
 }
 
-int Process::WaitPid(void)
-{
-#ifdef _MSC_VER
-	return _pclose(m_FP);
-#else /* _MSC_VER */
-	fclose(m_FP);
-
-	int status;
-	if (waitpid(m_Pid, &status, 0) != m_Pid)
-		BOOST_THROW_EXCEPTION(PosixException("waitpid() failed.", errno));
-
-	return status;
-#endif /* _MSC_VER */
-}
-
-void Process::InitTask(void)
-{
-	m_Result.ExecutionStart = Utility::GetTime();
-
-	Spawn(m_Command, m_Environment);
-}
-
 bool Process::RunTask(void)
 {
 	char buffer[512];
@@ -288,7 +273,14 @@ bool Process::RunTask(void)
 
 	int status, exitcode;
 
-	status = WaitPid();
+#ifdef _MSC_VER
+	status = _pclose(m_FP);
+#else /* _MSC_VER */
+	fclose(m_FP);
+
+	if (waitpid(m_Pid, &status, 0) != m_Pid)
+		BOOST_THROW_EXCEPTION(PosixException("waitpid() failed.", errno));
+#endif /* _MSC_VER */
 
 #ifndef _MSC_VER
 	if (WIFEXITED(status)) {
@@ -320,13 +312,3 @@ bool Process::RunTask(void)
 
 	return false;
 }
-
-/**
- * Retrieves the stdout file descriptor for the child process.
- *
- * @returns The stdout file descriptor.
- */
-int Process::GetFD(void) const
-{
-	return fileno(m_FP);
-}
diff --git a/lib/base/process.h b/lib/base/process.h
index 6044345..3d1e1df 100644
--- a/lib/base/process.h
+++ b/lib/base/process.h
@@ -50,13 +50,14 @@ public:
 
 	static const deque<Process::Ptr>::size_type MaxTasksPerThread = 512;
 
-	Process(const String& command, const Dictionary::Ptr& environment = Dictionary::Ptr());
+	Process(const vector<String>& arguments, const Dictionary::Ptr& extraEnvironment = Dictionary::Ptr());
 
+	static vector<String> ParseCommand(const String& command);
 private:
 	static bool m_ThreadCreated;
 
-	String m_Command;
-	Dictionary::Ptr m_Environment;
+	char **m_Arguments;
+	char **m_Environment;
 
 #ifndef _WIN32
 	pid_t m_Pid;
@@ -77,12 +78,6 @@ private:
 
 	void InitTask(void);
 	bool RunTask(void);
-
-	int GetFD(void) const;
-
-	void Spawn(const String& command, const Dictionary::Ptr& extraEnv);
-	void Spawn(const vector<String>& args, const Dictionary::Ptr& extraEnv);
-	int WaitPid(void);
 };
 
 }
diff --git a/lib/icinga/pluginchecktask.cpp b/lib/icinga/pluginchecktask.cpp
index 44ae326..a5a977c 100644
--- a/lib/icinga/pluginchecktask.cpp
+++ b/lib/icinga/pluginchecktask.cpp
@@ -48,7 +48,7 @@ void PluginCheckTask::ScriptFunc(const ScriptTask::Ptr& task, const vector<Value
 	macroDicts.push_back(IcingaApplication::GetInstance()->GetMacros());
 	String command = MacroProcessor::ResolveMacros(checkCommand, macroDicts);
 
-	Process::Ptr process = boost::make_shared<Process>(command, MacroProcessor::MakeEnvironment(macroDicts));
+	Process::Ptr process = boost::make_shared<Process>(Process::ParseCommand(command), MacroProcessor::MakeEnvironment(macroDicts));
 
 	PluginCheckTask ct(task, process);
 
diff --git a/lib/icinga/pluginnotificationtask.cpp b/lib/icinga/pluginnotificationtask.cpp
index 7bea524..90f0b36 100644
--- a/lib/icinga/pluginnotificationtask.cpp
+++ b/lib/icinga/pluginnotificationtask.cpp
@@ -53,7 +53,7 @@ void PluginNotificationTask::ScriptFunc(const ScriptTask::Ptr& task, const vecto
 	macroDicts.push_back(IcingaApplication::GetInstance()->GetMacros());
 	String command = MacroProcessor::ResolveMacros(notificationCommand, macroDicts);
 
-	Process::Ptr process = boost::make_shared<Process>(command, MacroProcessor::MakeEnvironment(macroDicts));
+	Process::Ptr process = boost::make_shared<Process>(Process::ParseCommand(command), MacroProcessor::MakeEnvironment(macroDicts));
 
 	PluginNotificationTask ct(task, process, notification->GetService()->GetName(), command);
 
diff --git a/lib/icinga/service-notification.cpp b/lib/icinga/service-notification.cpp
index 11e20b1..79f4d2d 100644
--- a/lib/icinga/service-notification.cpp
+++ b/lib/icinga/service-notification.cpp
@@ -35,7 +35,7 @@ void Service::RequestNotifications(NotificationType type) const
 	params.SetService(GetName());
 	params.SetType(type);
 
-	Logger::Write(LogInformation, "icinga", "Sending notification anycast request for service '" + GetName() + "'");
+	Logger::Write(LogDebug, "icinga", "Sending notification anycast request for service '" + GetName() + "'");
 	EndpointManager::GetInstance()->SendAnycastMessage(Endpoint::Ptr(), msg);
 }
 





More information about the icinga-checkins mailing list