[pacman-dev] [PATCH 2/2] [WIP] run XferCommand via exec

[Andrew Gregory]

---

systemvp should pretty much be a drop-in replacement for system with
the exception that it takes an argv array and uses exec.  If anybody
wants to play with it to stress test it a little, I have
a self-contained copy and test program at:
https://github.com/andrewgregory/snippets/blob/systemv/c/systemv.c

TODO:
    * update docs
    * fix debug logging
    * should the command be run with PATH lookup (execv vs execvp)?
    * Is the use of mmap with MAP_ANONYMOUS okay?  MAP_ANONYMOUS is
      not POSIX but "most systems also support MAP_ANONYMOUS (or its
      synonym MAP_ANON)" (mmap(2)).
    * should we reset signals prior to exec'ing like we do with
      hooks/scripts?

 src/pacman/conf.c | 95 ++++++++++++++++++++++++++++++++++++++---------
 src/pacman/conf.h |  2 +
 2 files changed, 80 insertions(+), 17 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index 2d8518c4..faf446dc 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -26,9 +26,11 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h> /* strdup */
+#include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/utsname.h> /* uname */
+#include <sys/wait.h>
 #include <unistd.h>
 
 /* pacman */
@@ -153,6 +155,7 @@ int config_free(config_t *oldconfig)
 	free(oldconfig->print_format);
 	free(oldconfig->arch);
 	free(oldconfig);
+	_alpm_wordsplit_free(oldconfig->xfercommand_argv);
 
 	return 0;
 }
@@ -201,18 +204,59 @@ static char *get_tempfile(const char *path, const char *filename)
 	return tempfile;
 }
 
+static int systemvp(const char *file, char *const argv[])
+{
+	int pid, *err;
+
+	err = mmap(NULL, sizeof(*err), PROT_READ | PROT_WRITE,
+			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	if(err == NULL) {
+		return -1;
+	}
+
+	pid = fork();
+	if(pid == -1) {
+		/* error */
+		munmap(err, sizeof(*err));
+		return -1;
+	} else if(pid == 0) {
+		/* child */
+		*err = 0;
+		execvp(file, argv);
+		*err = errno;
+		munmap(err, sizeof(*err));
+		_Exit(1);
+	} else {
+		/* parent */
+		int status;
+		while(waitpid(pid, &status, 0) == -1) {
+			if(errno != EINTR) {
+				munmap(err, sizeof(*err));
+				return -1;
+			}
+		}
+		if(*err != 0) {
+			errno = *err;
+			status = -1;
+		}
+		munmap(err, sizeof(*err));
+		return status;
+	}
+}
+
 /** External fetch callback */
 static int download_with_xfercommand(const char *url, const char *localpath,
 		int force)
 {
 	int ret = 0, retval;
 	int usepart = 0;
-	int cwdfd;
+	int cwdfd = -1;
 	struct stat st;
-	char *parsedcmd, *tempcmd;
 	char *destfile, *tempfile, *filename;
+	const char **argv;
+	size_t i;
 
-	if(!config->xfercommand) {
+	if(!config->xfercommand_argv) {
 		return -1;
 	}
 
@@ -230,17 +274,20 @@ static int download_with_xfercommand(const char *url, const char *localpath,
 		unlink(destfile);
 	}
 
-	tempcmd = strdup(config->xfercommand);
-	/* replace all occurrences of %o with fn.part */
-	if(strstr(tempcmd, "%o")) {
-		usepart = 1;
-		parsedcmd = strreplace(tempcmd, "%o", tempfile);
-		free(tempcmd);
-		tempcmd = parsedcmd;
+	if((argv = calloc(config->xfercommand_argc + 1, sizeof(char*))) == NULL) {
+		pm_printf(ALPM_LOG_ERROR, _("could not get current working directory\n"));
+		goto cleanup;
+	}
+
+	for(i = 0; i <= config->xfercommand_argc; i++) {
+		const char *val = config->xfercommand_argv[i];
+		if(val && strcmp(val, "%o") == 0) {
+			val = tempfile;
+		} else if(val && strcmp(val, "%u") == 0) {
+			val = url;
+		}
+		argv[i] = val;
 	}
-	/* replace all occurrences of %u with the download URL */
-	parsedcmd = strreplace(tempcmd, "%u", url);
-	free(tempcmd);
 
 	/* save the cwd so we can restore it later */
 	do {
@@ -256,9 +303,13 @@ static int download_with_xfercommand(const char *url, const char *localpath,
 		ret = -1;
 		goto cleanup;
 	}
-	/* execute the parsed command via /bin/sh -c */
-	pm_printf(ALPM_LOG_DEBUG, "running command: %s\n", parsedcmd);
-	retval = system(parsedcmd);
+
+	printf("running command: %s", config->xfercommand_argv[0]);
+	for(i = 1; argv[i]; i++) {
+		printf(" '%s'", argv[i]);
+	}
+	printf("\n");
+	retval = systemvp(argv[0], (char**)argv);
 
 	if(retval == -1) {
 		pm_printf(ALPM_LOG_WARNING, _("running XferCommand: fork failed!\n"));
@@ -296,7 +347,6 @@ cleanup:
 	}
 	free(destfile);
 	free(tempfile);
-	free(parsedcmd);
 
 	return ret;
 }
@@ -544,6 +594,17 @@ static int _parse_options(const char *key, char *value,
 				pm_printf(ALPM_LOG_DEBUG, "config: logfile: %s\n", value);
 			}
 		} else if(strcmp(key, "XferCommand") == 0) {
+			char **c;
+			if((config->xfercommand_argv = _alpm_wordsplit(value)) == NULL) {
+				pm_printf(ALPM_LOG_WARNING,
+						_("config file %s, line %d: invalid value for '%s' : '%s'\n"),
+						file, linenum, "XferCommand", value);
+				return 1;
+			}
+			config->xfercommand_argc = 0;
+			for(c = config->xfercommand_argv; *c; c++) {
+				config->xfercommand_argc++;
+			}
 			config->xfercommand = strdup(value);
 			pm_printf(ALPM_LOG_DEBUG, "config: xfercommand: %s\n", value);
 		} else if(strcmp(key, "CleanMethod") == 0) {
diff --git a/src/pacman/conf.h b/src/pacman/conf.h
index f45ed436..47df979d 100644
--- a/src/pacman/conf.h
+++ b/src/pacman/conf.h
@@ -125,6 +125,8 @@ typedef struct __config_t {
 	alpm_list_t *noextract;
 	alpm_list_t *overwrite_files;
 	char *xfercommand;
+	char **xfercommand_argv;
+	size_t xfercommand_argc;
 
 	/* our connection to libalpm */
 	alpm_handle_t *handle;
-- 
2.21.0