[Cluster-devel] conga/ricci common/executils.cpp modules/log/L ...

[rmccabe at sourceware.org]

CVSROOT:	/cvs/cluster
Module name:	conga
Changes by:	rmccabe at sourceware.org	2007-09-20 05:39:04

Modified files:
	ricci/common   : executils.cpp 
	ricci/modules/log: LogParser.cpp 

Log message:
	Ensure that malloc/new/whatever is not called between fork() and execv() in a function that can be called from a thread.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/conga/ricci/common/executils.cpp.diff?cvsroot=cluster&r1=1.11&r2=1.12
http://sourceware.org/cgi-bin/cvsweb.cgi/conga/ricci/modules/log/LogParser.cpp.diff?cvsroot=cluster&r1=1.12&r2=1.13

--- conga/ricci/common/executils.cpp	2007/09/18 21:01:00	1.11
+++ conga/ricci/common/executils.cpp	2007/09/20 05:39:03	1.12
@@ -52,17 +52,42 @@
 	int _stdout_pipe[2];
 	int _stderr_pipe[2];
 
-	if (pipe(_stdout_pipe) == -1)
+	/*
+	** Prepare argv before forking in case we were called from a thread.
+	*/
+	unsigned int size = args.size() + 2;
+	char **argv = (char **) malloc(sizeof(char *) * size);
+	if (argv == NULL)
+		return 3;
+
+	try {
+		argv[0] = (char *) path.c_str();
+		for (unsigned int i = 0 ; i < args.size() ; i++)
+			argv[i + 1] = (char *) args[i].c_str();
+		argv[size - 1] = NULL;
+	} catch (...) {
+		free(argv);
+		return 3;
+	}
+
+	if (pipe(_stdout_pipe) == -1) {
+		free(argv);
 		return 2;
+	}
 
 	if (pipe(_stderr_pipe) == -1) {
+		free(argv);
 		close_fd(_stdout_pipe[0]);
 		close_fd(_stdout_pipe[1]);
 		return 2;
 	}
 
+	setenv("LANG", "C", 1);
+	setenv("LC_ALL", "C", 1);
+
 	int pid = fork();
 	if (pid == -1) {
+		free(argv);
 		close_fd(_stdout_pipe[0]);
 		close_fd(_stdout_pipe[1]);
 		close_fd(_stderr_pipe[0]);
@@ -70,9 +95,6 @@
 		return 3;
 	}
 
-	unsigned int time_beg = time_mil();
-	unsigned int time_to_kill = time_beg + timeout;
-
 	if (pid == 0) {
 		/* child */
 
@@ -97,7 +119,7 @@
 		close_fd(devnull);
 
 		// close open fds
-		for (unsigned int i = 3; i < __FD_SETSIZE ; i++)
+		for (unsigned int i = 3 ; i < __FD_SETSIZE ; i++)
 			close_fd(i);
 
 		// restore signals
@@ -108,27 +130,18 @@
 		sigfillset(&set);
 		sigprocmask(SIG_UNBLOCK, &set, NULL);
 
-		setenv("LANG", "C", 1);
-		setenv("LC_ALL", "C", 1);
-
-		/* exec */
-		try {
-			unsigned int size = args.size() + 2;
-			char **argv = new char*[size];
-
-			argv[0] = (char *) path.c_str();
-			for (unsigned int i = 0 ; i < args.size() ; i++)
-				argv[i + 1] = (char *) args[i].c_str();
-			argv[size - 1] = NULL;
-			execv(path.c_str(), argv);
-		} catch ( ... ) {}
+		execv(path.c_str(), argv);
 		_exit(1);
 	}
 
 	/* parent */
+	unsigned int time_beg = time_mil();
+	unsigned int time_to_kill = time_beg + timeout;
+	bool out_closed = false, err_closed = false;
+
+	free(argv);
 	close_fd(_stdout_pipe[1]);
 	close_fd(_stderr_pipe[1]);
-	bool out_closed = false, err_closed = false;
 
 	while (true) {
 		// kill child if timeout elapsed
@@ -183,12 +196,12 @@
 			if (poll_info.fd == _stderr_pipe[0])
 				read_data(poll_info, err_closed, err);
 		}
-	} // while (true)
+	}
 
 	// command
-	String comm(path);
-	for (unsigned int i = 0 ; i < args.size() ; i++)
-		comm += " " + args[i];
+	//String comm(path);
+	//for (unsigned int i = 0 ; i < args.size() ; i++)
+	//	comm += " " + args[i];
 
 	// get status
 	int ret;
--- conga/ricci/modules/log/LogParser.cpp	2007/09/18 21:01:00	1.12
+++ conga/ricci/modules/log/LogParser.cpp	2007/09/20 05:39:03	1.13
@@ -437,12 +437,8 @@
 				log.getline(buff, sizeof(buff));
 				LogEntry e(buff);
 
-				/*
-				** Since the log entries' times ascend chronologically, we
-				** can bail if we hit an entry that's older than the max age.
-				*/
 				if (e.age > age)
-					break;
+					continue;
 
 				if (req_tags.empty())
 					ret.insert(e);