[pacman-dev] [PATCH 1/2] Refactor _alpm_runscriptlet()

Dan McGee dan at archlinux.org
Fri Sep 30 11:50:32 EDT 2011


Add an is_archive parameter to reduce the amount of black magic going
on. Rework to use fewer PATH_MAX sized local variables, and simplify
some of the logic where appropriate in both this function and in the
callers where duplicate calls can be replaced by some conditional
parameter code.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/add.c    |   48 +++++++++++++++++---------------------------
 lib/libalpm/remove.c |    4 +-
 lib/libalpm/trans.c  |   53 +++++++++++++++++++++++++++++++++-----------------
 lib/libalpm/trans.h  |    4 +-
 4 files changed, 58 insertions(+), 51 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 170d09e..33b85c0 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -452,17 +452,13 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg,
 		size_t pkg_current, size_t pkg_count)
 {
 	int i, ret = 0, errors = 0;
-	char scriptlet[PATH_MAX];
-	int is_upgrade = 0;
+	int is_upgrade;
 	alpm_pkg_t *oldpkg = NULL;
 	alpm_db_t *db = handle->db_local;
 	alpm_trans_t *trans = handle->trans;
 
 	ASSERT(trans != NULL, return -1);
 
-	snprintf(scriptlet, PATH_MAX, "%s%s-%s/install",
-			_alpm_db_path(db), newpkg->name, newpkg->version);
-
 	/* see if this is an upgrade. if so, remove the old package first */
 	alpm_pkg_t *local = _alpm_db_get_pkgfromcache(db, newpkg->name);
 	if(local) {
@@ -474,30 +470,23 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg,
 			goto cleanup;
 		}
 
-		EVENT(handle, ALPM_EVENT_UPGRADE_START, newpkg, local);
-		_alpm_log(handle, ALPM_LOG_DEBUG, "upgrading package %s-%s\n",
-				newpkg->name, newpkg->version);
-
 		/* copy over the install reason */
 		newpkg->reason = alpm_pkg_get_reason(local);
 
-		/* pre_upgrade scriptlet */
-		if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) {
-			_alpm_runscriptlet(handle, newpkg->origin_data.file,
-					"pre_upgrade", newpkg->version, local->version);
-		}
+		EVENT(handle, ALPM_EVENT_UPGRADE_START, newpkg, local);
 	} else {
 		is_upgrade = 0;
-
 		EVENT(handle, ALPM_EVENT_ADD_START, newpkg, NULL);
-		_alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s-%s\n",
-				newpkg->name, newpkg->version);
+	}
 
-		/* pre_install scriptlet */
-		if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) {
-			_alpm_runscriptlet(handle, newpkg->origin_data.file,
-					"pre_install", newpkg->version, NULL);
-		}
+	_alpm_log(handle, ALPM_LOG_DEBUG, "%s package %s-%s\n",
+			is_upgrade ? "upgrading" : "adding", newpkg->name, newpkg->version);
+		/* pre_install/pre_upgrade scriptlet */
+	if(alpm_pkg_has_scriptlet(newpkg) &&
+			!(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) {
+		const char *scriptlet_name = is_upgrade ? "pre_upgrade" : "pre_install";
+		_alpm_runscriptlet(handle, newpkg->origin_data.file,
+				scriptlet_name, newpkg->version, NULL, 1);
 	}
 
 	/* we override any pre-set reason if we have alldeps or allexplicit set */
@@ -661,13 +650,14 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg,
 	/* run the post-install script if it exists  */
 	if(alpm_pkg_has_scriptlet(newpkg)
 			&& !(trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) {
-		if(is_upgrade) {
-			_alpm_runscriptlet(handle, scriptlet, "post_upgrade",
-					newpkg->version, oldpkg ? oldpkg->version : NULL);
-		} else {
-			_alpm_runscriptlet(handle, scriptlet, "post_install",
-					newpkg->version, NULL);
-		}
+		char scriptlet[PATH_MAX];
+		const char *scriptlet_name;
+		snprintf(scriptlet, PATH_MAX, "%s%s-%s/install",
+				_alpm_db_path(db), newpkg->name, newpkg->version);
+		scriptlet_name = is_upgrade ? "post_upgrade" : "post_install";
+
+		_alpm_runscriptlet(handle, scriptlet, scriptlet_name,
+				newpkg->version, oldpkg ? oldpkg->version : NULL, 0);
 	}
 
 	if(is_upgrade) {
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index a3aa4a5..7f917fa 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -376,7 +376,7 @@ int _alpm_remove_single_package(alpm_handle_t *handle,
 		/* run the pre-remove scriptlet if it exists  */
 		if(alpm_pkg_has_scriptlet(oldpkg) &&
 				!(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) {
-			_alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL);
+			_alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL, 0);
 		}
 	}
 
@@ -453,7 +453,7 @@ int _alpm_remove_single_package(alpm_handle_t *handle,
 		/* run the post-remove script if it exists  */
 		if(alpm_pkg_has_scriptlet(oldpkg) &&
 				!(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) {
-			_alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL);
+			_alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL, 0);
 		}
 	}
 
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
index fbc5fee..cb97a4a 100644
--- a/lib/libalpm/trans.c
+++ b/lib/libalpm/trans.c
@@ -275,40 +275,50 @@ static int grep(const char *fn, const char *needle)
 	return 0;
 }
 
-int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn,
-		const char *script, const char *ver, const char *oldver)
+int _alpm_runscriptlet(alpm_handle_t *handle, const char *filepath,
+		const char *script, const char *ver, const char *oldver, int is_archive)
 {
-	char scriptfn[PATH_MAX];
 	char cmdline[PATH_MAX];
-	char tmpdir[PATH_MAX];
 	char *argv[] = { "sh", "-c", cmdline, NULL };
-	char *scriptpath;
+	char *tmpdir, *scriptfn = NULL, *scriptpath;
 	int retval = 0;
+	size_t len;
 
-	if(_alpm_access(handle, NULL, installfn, R_OK) != 0) {
-		_alpm_log(handle, ALPM_LOG_DEBUG, "scriptlet '%s' not found\n", installfn);
+	if(_alpm_access(handle, NULL, filepath, R_OK) != 0) {
+		_alpm_log(handle, ALPM_LOG_DEBUG, "scriptlet '%s' not found\n", filepath);
 		return 0;
 	}
 
-	/* creates a directory in $root/tmp/ for copying/extracting the scriptlet */
-	snprintf(tmpdir, PATH_MAX, "%stmp/", handle->root);
+	if(!is_archive && !grep(filepath, script)) {
+		/* script not found in scriptlet file; we can only short-circuit this early
+		 * if it is an actual scriptlet file and not an archive.  */
+		return 0;
+	}
+
+	/* create a directory in $root/tmp/ for copying/extracting the scriptlet */
+	len = strlen(handle->root) + strlen("tmp/alpm_XXXXXX") + 1;
+	MALLOC(tmpdir, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+	snprintf(tmpdir, len, "%stmp/", handle->root);
 	if(access(tmpdir, F_OK) != 0) {
 		_alpm_makepath_mode(tmpdir, 01777);
 	}
-	snprintf(tmpdir, PATH_MAX, "%stmp/alpm_XXXXXX", handle->root);
+	snprintf(tmpdir, len, "%stmp/alpm_XXXXXX", handle->root);
 	if(mkdtemp(tmpdir) == NULL) {
 		_alpm_log(handle, ALPM_LOG_ERROR, _("could not create temp directory\n"));
+		free(tmpdir);
 		return 1;
 	}
 
 	/* either extract or copy the scriptlet */
-	snprintf(scriptfn, PATH_MAX, "%s/.INSTALL", tmpdir);
-	if(strcmp(script, "pre_upgrade") == 0 || strcmp(script, "pre_install") == 0) {
-		if(_alpm_unpack_single(handle, installfn, tmpdir, ".INSTALL")) {
+	len += strlen("/.INSTALL");
+	MALLOC(scriptfn, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+	snprintf(scriptfn, len, "%s/.INSTALL", tmpdir);
+	if(is_archive) {
+		if(_alpm_unpack_single(handle, filepath, tmpdir, ".INSTALL")) {
 			retval = 1;
 		}
 	} else {
-		if(_alpm_copyfile(installfn, scriptfn)) {
+		if(_alpm_copyfile(filepath, scriptfn)) {
 			_alpm_log(handle, ALPM_LOG_ERROR, _("could not copy tempfile to %s (%s)\n"), scriptfn, strerror(errno));
 			retval = 1;
 		}
@@ -317,8 +327,8 @@ int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn,
 		goto cleanup;
 	}
 
-	if(!grep(scriptfn, script)) {
-		/* script not found in scriptlet file */
+	if(is_archive && !grep(scriptfn, script)) {
+		/* script not found in extracted scriptlet file */
 		goto cleanup;
 	}
 
@@ -338,10 +348,17 @@ int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn,
 	retval = _alpm_run_chroot(handle, "/bin/sh", argv);
 
 cleanup:
-	if(_alpm_rmrf(tmpdir)) {
-		_alpm_log(handle, ALPM_LOG_WARNING, _("could not remove tmpdir %s\n"), tmpdir);
+	if(scriptfn && unlink(scriptfn)) {
+		_alpm_log(handle, ALPM_LOG_WARNING,
+				_("could not remove %s\n"), scriptfn);
+	}
+	if(rmdir(tmpdir)) {
+		_alpm_log(handle, ALPM_LOG_WARNING,
+				_("could not remove tmpdir %s\n"), tmpdir);
 	}
 
+	free(scriptfn);
+	free(tmpdir);
 	return retval;
 }
 
diff --git a/lib/libalpm/trans.h b/lib/libalpm/trans.h
index 38f45ff..ff944b0 100644
--- a/lib/libalpm/trans.h
+++ b/lib/libalpm/trans.h
@@ -47,8 +47,8 @@ struct __alpm_trans_t {
 
 void _alpm_trans_free(alpm_trans_t *trans);
 int _alpm_trans_init(alpm_trans_t *trans, alpm_transflag_t flags);
-int _alpm_runscriptlet(alpm_handle_t *handle, const char *installfn,
-		const char *script, const char *ver, const char *oldver);
+int _alpm_runscriptlet(alpm_handle_t *handle, const char *filepath,
+		const char *script, const char *ver, const char *oldver, int is_archive);
 
 #endif /* _ALPM_TRANS_H */
 
-- 
1.7.6.4



More information about the pacman-dev mailing list