Cleanup

bin/xbps-install/transaction.c

@@ -28,7 +28,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
-#include <ctype.h>
 #include <assert.h>
 #include <unistd.h>
 #include <limits.h>
@@ -317,9 +316,7 @@ dist_upgrade(struct xbps_handle *xhp, unsigned int cols, bool yes, bool drun)
 		xbps_error_printf("No packages currently registered.\n");
 		return 0;
 	} else if (rv == EBUSY) {
-		if (drun) {
-			rv = 0;
-		} else {
+		if (!drun) {
 			xbps_error_printf("The 'xbps' package must be updated, please run `xbps-install -u xbps`\n");
 			return rv;
 		}

include/xbps.h.in

@@ -777,6 +777,16 @@ int xbps_error_errno(int r, const char *fmt, ...)  PRINTF_LIKE(2, 3);
 	xbps_error_errno(ENOMEM, "%s:%d %s: out of memory\n", __FILE__,        \
 	        __LINE__, __func__)
 
+/**
+ * @brief Log and abort for code that should be unreachable.
+ */
+#define xbps_unreachable()                                                     \
+	do {                                                                   \
+		xbps_error_printf("%s:%d %s: code should not be reached\n",    \
+		    __FILE__, __LINE__, __func__);                             \
+		abort();                                                       \
+	} while (0)
+
 /**@}*/
 
 /** @addtogroup configure */

lib/package_remove.c

@@ -55,28 +55,24 @@ check_remove_pkg_files(struct xbps_handle *xhp,
 		 * Check if effective user ID owns the file; this is
 		 * enough to ensure the user has write permissions
 		 * on the directory.
+		 * XXX: this isn't really correct, would need to check parent
+		 * directory permissions...
 		 */
-		if (lstat(file, &st) == 0 && euid == st.st_uid) {
-			/* success */
-			continue;
+		if (lstat(file, &st) == 0) {
+			if (euid == st.st_uid)
+				continue;
+			errno = EPERM;
 		}
 		if (errno != ENOENT) {
 			/*
 			 * only bail out if something else than ENOENT
 			 * is returned.
 			 */
-			int rv = errno;
-			if (rv == 0) {
-				/* lstat succeeds but euid != uid */
-				rv = EPERM;
-			}
 			fail = true;
 			xbps_set_cb_state(xhp, XBPS_STATE_REMOVE_FILE_FAIL,
-				rv, pkgver,
-				"%s: cannot remove `%s': %s",
-				pkgver, file, strerror(rv));
+			    errno, pkgver, "%s: cannot remove `%s': %s", pkgver,
+			    file, strerror(errno));
 		}
-		errno = 0;
 	}
 	return fail;
 }

lib/package_unpack.c

@@ -88,7 +88,6 @@ unpack_archive(struct xbps_handle *xhp,
 {
 	xbps_dictionary_t binpkg_filesd, pkg_filesd, obsd;
 	xbps_array_t array, obsoletes;
-	xbps_trans_type_t ttype;
 	const struct stat *entry_statp;
 	struct stat st;
 	struct xbps_unpack_cb_data xucd;
@@ -97,17 +96,16 @@ unpack_archive(struct xbps_handle *xhp,
 	const char *entry_pname, *pkgname;
 	const char *file;
 	int ar_rv, rv, error, entry_type, flags;
-	bool preserve, update, file_exists, keep_conf_file;
+	bool preserve, file_exists, keep_conf_file;
 	bool skip_extract, force, xucd_stats;
 	uid_t euid;
 
 	binpkg_filesd = pkg_filesd = NULL;
-	force = preserve = update = file_exists = false;
+	force = preserve = false;
 	xucd_stats = false;
-	ar_rv = rv = error = entry_type = flags = 0;
+	ar_rv = rv = error = 0;
 
 	xbps_dictionary_get_bool(pkg_repod, "preserve", &preserve);
-	ttype = xbps_transaction_pkg_type(pkg_repod);
 
 	memset(&xucd, 0, sizeof(xucd));
 
@@ -121,10 +119,6 @@ unpack_archive(struct xbps_handle *xhp,
 		force = true;
 	}
 
-	if (ttype == XBPS_TRANS_UPDATE) {
-		update = true;
-	}
-
 	/*
 	 * Remove obsolete files.
 	 */
@@ -169,7 +163,6 @@ unpack_archive(struct xbps_handle *xhp,
 			break;
 
 		entry_pname = archive_entry_pathname(entry);
-		entry_size = archive_entry_size(entry);
 
 		if (strcmp("./INSTALL", entry_pname) == 0 ||
 		    strcmp("./REMOVE", entry_pname) == 0 ||

lib/transaction_commit.c

@@ -28,7 +28,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
-#include <ctype.h>
 #include <unistd.h>
 #include <limits.h>
 #include <locale.h>
@@ -109,7 +108,6 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 	xbps_trans_type_t ttype;
 	const char *pkgver = NULL, *pkgname = NULL;
 	int rv = 0;
-	bool update, replaced;
 
 	setlocale(LC_ALL, "");
 
@@ -220,6 +218,7 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 		xbps_dictionary_t dict;
 		xbps_data_t script = NULL;
 		const char *pkgdb_pkgver;
+		bool update;
 
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgname", &pkgname);
@@ -232,7 +231,7 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 		}
 
 		if ((pkgdb_pkgd = xbps_pkgdb_get_pkg(xhp, pkgname)) == NULL) {
-			replaced = false;
+			bool replaced = false;
 			xbps_dictionary_get_bool(obj, "replaced", &replaced);
 			if (replaced) {
 				continue;
@@ -311,14 +310,14 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 
 		ttype = xbps_transaction_pkg_type(obj);
 		if (ttype == XBPS_TRANS_REMOVE) {
+			bool replaced = false;
+			bool update = false;
 			/*
 			 * Remove package.
 			 */
-			update = false;
 			xbps_dictionary_get_bool(obj, "remove-and-update", &update);
-			replaced = false;
 			xbps_dictionary_get_bool(obj, "replaced", &replaced);
-			if (((pkgdb_pkgd = xbps_pkgdb_get_pkg(xhp, pkgname)) == NULL) && replaced) {
+			if (replaced && !xbps_pkgdb_get_pkg(xhp, pkgname)) {
 				continue;
 			}
 			rv = xbps_remove_pkg(xhp, pkgver, update);
@@ -410,16 +409,16 @@ xbps_transaction_commit(struct xbps_handle *xhp)
 	xbps_set_cb_state(xhp, XBPS_STATE_TRANS_CONFIGURE, 0, NULL, NULL);
 
 	while ((obj = xbps_object_iterator_next(iter)) != NULL) {
+		bool update;
+
 		xbps_dictionary_get_cstring_nocopy(obj, "pkgver", &pkgver);
 		ttype = xbps_transaction_pkg_type(obj);
 		if (ttype == XBPS_TRANS_REMOVE || ttype == XBPS_TRANS_HOLD) {
 			xbps_dbg_printf("%s: skipping configuration for "
 			    "%s: %d\n", __func__, pkgver, ttype);
 			continue;
 		}
-		update = false;
-		if (ttype == XBPS_TRANS_UPDATE)
-			update = true;
+		update = ttype == XBPS_TRANS_UPDATE;
 
 		rv = xbps_configure_pkg(xhp, pkgver, false, update);
 		if (rv != 0) {

lib/transaction_fetch.c

@@ -26,7 +26,6 @@
 
 #include <errno.h>
 #include <limits.h>
-#include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
@@ -122,31 +121,28 @@ download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
 	xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD, 0, pkgver,
 		"Downloading `%s' signature (from `%s')...", pkgver, repoloc);
 
-	if ((rv = xbps_fetch_file(xhp, buf, NULL)) == -1) {
+	if (xbps_fetch_file(xhp, buf, NULL) == -1) {
 		rv = fetchLastErrCode ? fetchLastErrCode : errno;
 		fetchstr = xbps_fetch_error_string();
 		xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD_FAIL, rv,
 			pkgver, "[trans] failed to download `%s' signature from `%s': %s",
 			pkgver, repoloc, fetchstr ? fetchstr : strerror(rv));
 		return rv;
 	}
-	rv = 0;
 
 	*sigsuffix = '\0';
 
 	xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD, 0, pkgver,
 		"Downloading `%s' package (from `%s')...", pkgver, repoloc);
 
-	if ((rv = xbps_fetch_file_sha256(xhp, buf, NULL, digest,
-	    sizeof digest)) == -1) {
+	if (xbps_fetch_file_sha256(xhp, buf, NULL, digest, sizeof digest) == -1) {
 		rv = fetchLastErrCode ? fetchLastErrCode : errno;
 		fetchstr = xbps_fetch_error_string();
 		xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD_FAIL, rv,
 			pkgver, "[trans] failed to download `%s' package from `%s': %s",
 			pkgver, repoloc, fetchstr ? fetchstr : strerror(rv));
 		return rv;
 	}
-	rv = 0;
 
 	xbps_set_cb_state(xhp, XBPS_STATE_VERIFY, 0, pkgver,
 		"%s: verifying RSA signature...", pkgver);
@@ -165,6 +161,7 @@ download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
 	 * If digest is not set, binary package was not downloaded,
 	 * i.e. 304 not modified, verify by file instead.
 	 */
+	rv = 0;
 	if (fetchLastErrCode == FETCH_UNCHANGED) {
 		*sigsuffix = '\0';
 		if (!xbps_verify_file_signature(repo, buf)) {

lib/transaction_ops.c

@@ -392,17 +392,14 @@ xbps_transaction_update_pkg(struct xbps_handle *xhp, const char *pkg, bool force
 		rdeps = NULL;
 	}
 	for (unsigned int i = 0; i < xbps_array_count(rdeps); i++)  {
+		char pkgname[XBPS_NAME_SIZE];
 		const char *pkgver = NULL;
-		char pkgname[XBPS_NAME_SIZE] = {0};
 
-		if (!xbps_array_get_cstring_nocopy(rdeps, i, &pkgver)) {
-			rv = EINVAL;
-			break;
-		}
-		if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver)) {
-			rv = EINVAL;
-			break;
-		}
+		if (!xbps_array_get_cstring_nocopy(rdeps, i, &pkgver))
+			xbps_unreachable();
+		if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver))
+			xbps_unreachable();
+
 		rv = trans_find_pkg(xhp, pkgname, false);
 		xbps_dbg_printf("%s: trans_find_pkg %s: %d\n", __func__, pkgver, rv);
 		if (rv && rv != ENOENT && rv != EEXIST && rv != ENODEV) {
@@ -442,17 +439,14 @@ xbps_transaction_install_pkg(struct xbps_handle *xhp, const char *pkg, bool forc
 		rdeps = NULL;
 	}
 	for (unsigned int i = 0; i < xbps_array_count(rdeps); i++)  {
+		char pkgname[XBPS_NAME_SIZE];
 		const char *pkgver = NULL;
-		char pkgname[XBPS_NAME_SIZE] = {0};
 
-		if (!xbps_array_get_cstring_nocopy(rdeps, i, &pkgver)) {
-			rv = EINVAL;
-			break;
-		}
-		if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver)) {
-			rv = EINVAL;
-			break;
-		}
+		if (!xbps_array_get_cstring_nocopy(rdeps, i, &pkgver))
+			xbps_unreachable();
+		if (!xbps_pkg_name(pkgname, sizeof(pkgname), pkgver))
+			xbps_unreachable();
+
 		rv = trans_find_pkg(xhp, pkgname, false);
 		xbps_dbg_printf("%s: trans_find_pkg %s: %d\n", __func__, pkgver, rv);
 		if (rv && rv != ENOENT && rv != EEXIST && rv != ENODEV) {