[pacman-dev] [PATCH] Refactor the trans init and release code.

26 Apr 2008

The calls to alpm_trans_init and alpm_trans_release (+ error checking) were
duplicated between remove.c, sync.c and upgrade.c
This patch introduces trans_init and trans_release functions in util.c to
have this code just once.

So instead of having to do the same change 3 times for fixing FS#10273, I
just had to do it once (so I did it too :))

Signed-off-by: Chantry Xavier <shiningxc@gmail.com>
---
 src/pacman/remove.c  |   35 ++++++++---------------------------
 src/pacman/sync.c    |   39 ++++++++-------------------------------
 src/pacman/upgrade.c |   34 +++++++---------------------------
 src/pacman/util.c    |   28 ++++++++++++++++++++++++++++
 src/pacman/util.h    |    2 ++
 5 files changed, 53 insertions(+), 85 deletions(-)

diff --git a/src/pacman/remove.c b/src/pacman/remove.c
index 2fb69d3..a2209ac 100644
--- a/src/pacman/remove.c
+++ b/src/pacman/remove.c
@@ -29,24 +29,10 @@
 /* pacman */
 #include "pacman.h"
 #include "util.h"
-#include "callback.h"
 #include "conf.h"
 
 extern pmdb_t *db_local;
 
-/* Free the current transaction and print an error if unsuccessful */
-static int remove_cleanup(void)
-{
-	int ret = alpm_trans_release();
-	if(ret != 0) {
-		pm_printf(PM_LOG_ERROR, _("failed to release transaction (%s)\n"),
-		        alpm_strerrorlast());
-		ret = 1;
-	}
-
-	return(ret);
-}
-
 /**
  * @brief Remove a specified list of packages.
  *
@@ -90,14 +76,7 @@ int pacman_remove(alpm_list_t *targets)
 	}
 
 	/* Step 1: create a new transaction */
-	if(alpm_trans_init(PM_TRANS_TYPE_REMOVE, config->flags,
-	   cb_trans_evt, cb_trans_conv, cb_trans_progress) == -1) {
-		fprintf(stderr, _("error: failed to init transaction (%s)\n"),
-		        alpm_strerrorlast());
-		if(pm_errno == PM_ERR_HANDLE_LOCK) {
-			printf(_("  if you're sure a package manager is not already\n"
-			         "  running, you can remove %s.\n"), alpm_option_get_lockfile());
-		}
+	if(trans_init(PM_TRANS_TYPE_REMOVE, config->flags) == -1) {
 		FREELIST(finaltargs);
 		return(1);
 	}
@@ -109,7 +88,7 @@ int pacman_remove(alpm_list_t *targets)
 		if(alpm_trans_addtarget(targ) == -1) {
 			fprintf(stderr, _("error: '%s': %s\n"),
 					targ, alpm_strerrorlast());
-			remove_cleanup();
+			trans_release();
 			FREELIST(finaltargs);
 			return(1);
 		}
@@ -134,7 +113,7 @@ int pacman_remove(alpm_list_t *targets)
 			default:
 				break;
 		}
-		remove_cleanup();
+		trans_release();
 		FREELIST(finaltargs);
 		return(1);
 	}
@@ -154,7 +133,7 @@ int pacman_remove(alpm_list_t *targets)
 		FREELIST(lst);
 		/* get confirmation */
 		if(yesno(1, _("\nDo you want to remove these packages?")) == 0) {
-			remove_cleanup();
+			trans_release();
 			FREELIST(finaltargs);
 			return(1);
 		}
@@ -165,13 +144,15 @@ int pacman_remove(alpm_list_t *targets)
 	if(alpm_trans_commit(NULL) == -1) {
 		fprintf(stderr, _("error: failed to commit transaction (%s)\n"),
 		        alpm_strerrorlast());
-		remove_cleanup();
+		trans_release();
 		FREELIST(finaltargs);
 		return(1);
 	}
 
 	/* Step 4: release transaction resources */
-	retval = remove_cleanup();
+	if(trans_release() == -1) {
+		retval = 1;
+	}
 	FREELIST(finaltargs);
 	return(retval);
 }
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index c074746..77b66da 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -34,7 +34,6 @@
 #include "pacman.h"
 #include "util.h"
 #include "package.h"
-#include "callback.h"
 #include "conf.h"
 
 extern pmdb_t *db_local;
@@ -235,34 +234,12 @@ static int sync_cleancache(int level)
 	return(0);
 }
 
-static int sync_trans_init(pmtransflag_t flags) {
-	if(alpm_trans_init(PM_TRANS_TYPE_SYNC, flags, cb_trans_evt,
-				cb_trans_conv, cb_trans_progress) == -1) {
-		fprintf(stderr, _("error: failed to init transaction (%s)\n"),
-				alpm_strerrorlast());
-		if(pm_errno == PM_ERR_HANDLE_LOCK) {
-			printf(_("  if you're sure a package manager is not already\n"
-						"  running, you can remove %s.\n"), alpm_option_get_lockfile());
-		}
-		return(-1);
-	}
-	return(0);
-}
-
-static int sync_trans_release() {
-	if(alpm_trans_release() == -1) {
-		fprintf(stderr, _("error: failed to release transaction (%s)\n"),
-				alpm_strerrorlast());
-		return(-1);
-	}
-	return(0);
-}
 static int sync_synctree(int level, alpm_list_t *syncs)
 {
 	alpm_list_t *i;
 	int success = 0, ret;
 
-	if(sync_trans_init(0) == -1) {
+	if(trans_init(PM_TRANS_TYPE_SYNC, 0) == -1) {
 		return(0);
 	}
 
@@ -281,7 +258,7 @@ static int sync_synctree(int level, alpm_list_t *syncs)
 		}
 	}
 
-	if(sync_trans_release() == -1) {
+	if(trans_release() == -1) {
 		return(0);
 	}
 	/* We should always succeed if at least one DB was upgraded - we may possibly
@@ -552,7 +529,7 @@ static int sync_trans(alpm_list_t *targets)
 	alpm_list_t *sync_dbs = alpm_option_get_syncdbs();
 
 	/* Step 1: create a new transaction... */
-	if(sync_trans_init(config->flags) == -1) {
+	if(trans_init(PM_TRANS_TYPE_SYNC, config->flags) == -1) {
 		return(1);
 	}
 
@@ -583,10 +560,10 @@ static int sync_trans(alpm_list_t *targets)
 					printf(_(":: pacman has detected a newer version of itself.\n"));
 					if(yesno(1, _(":: Do you want to cancel the current operation\n"
 					           ":: and install the new pacman version now?"))) {
-						if(sync_trans_release() == -1) {
+						if(trans_release() == -1) {
 							return(1);
 						}
-						if(sync_trans_init(0) == -1) {
+						if(trans_init(PM_TRANS_TYPE_SYNC, 0) == -1) {
 							return(1);
 						}
 						if(alpm_trans_addtarget("pacman") == -1) {
@@ -788,7 +765,7 @@ cleanup:
 	if(data) {
 		FREELIST(data);
 	}
-	if(sync_trans_release() == -1) {
+	if(trans_release() == -1) {
 		retval = 1;
 	}
 
@@ -803,14 +780,14 @@ int pacman_sync(alpm_list_t *targets)
 	if(config->op_s_clean) {
 		int ret = 0;
 
-		if(sync_trans_init(0) == -1) {
+		if(trans_init(PM_TRANS_TYPE_SYNC, 0) == -1) {
 			return(1);
 		}
 
 		ret += sync_cleancache(config->op_s_clean);
 		ret += sync_cleandb_all();
 
-		if(sync_trans_release() == -1) {
+		if(trans_release() == -1) {
 			ret++;
 		}
 
diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
index e562651..589970e 100644
--- a/src/pacman/upgrade.c
+++ b/src/pacman/upgrade.c
@@ -28,23 +28,9 @@
 
 /* pacman */
 #include "pacman.h"
-#include "callback.h"
 #include "conf.h"
 #include "util.h"
 
-/* Free the current transaction and print an error if unsuccessful */
-static int upgrade_cleanup(void)
-{
-	int ret = alpm_trans_release();
-	if(ret != 0) {
-		pm_printf(PM_LOG_ERROR, _("failed to release transaction (%s)\n"),
-		        alpm_strerrorlast());
-		ret = 1;
-	}
-
-	return(ret);
-}
-
 /**
  * @brief Upgrade a specified list of packages.
  *
@@ -78,15 +64,7 @@ int pacman_upgrade(alpm_list_t *targets)
 	}
 
 	/* Step 1: create a new transaction */
-	if(alpm_trans_init(transtype, config->flags, cb_trans_evt,
-	   cb_trans_conv, cb_trans_progress) == -1) {
-		/* TODO: error messages should be in the front end, not the back */
-		fprintf(stderr, _("error: %s\n"), alpm_strerrorlast());
-		if(pm_errno == PM_ERR_HANDLE_LOCK) {
-			/* TODO this and the 2 other places should probably be on stderr */
-			printf(_("  if you're sure a package manager is not already\n"
-			         "  running, you can remove %s.\n"), alpm_option_get_lockfile());
-		}
+	if(trans_init(transtype, config->flags) == -1) {
 		return(1);
 	}
 
@@ -97,7 +75,7 @@ int pacman_upgrade(alpm_list_t *targets)
 		if(alpm_trans_addtarget(targ) == -1) {
 			fprintf(stderr, _("error: '%s': %s\n"),
 					targ, alpm_strerrorlast());
-			upgrade_cleanup();
+			trans_release();
 			return(1);
 		}
 	}
@@ -151,7 +129,7 @@ int pacman_upgrade(alpm_list_t *targets)
 			default:
 				break;
 		}
-		upgrade_cleanup();
+		trans_release();
 		FREELIST(data);
 		return(1);
 	}
@@ -160,11 +138,13 @@ int pacman_upgrade(alpm_list_t *targets)
 	/* Step 3: perform the installation */
 	if(alpm_trans_commit(NULL) == -1) {
 		fprintf(stderr, _("error: failed to commit transaction (%s)\n"), alpm_strerrorlast());
-		upgrade_cleanup();
+		trans_release();
 		return(1);
 	}
 
-	retval = upgrade_cleanup();
+	if(trans_release() == -1) {
+		retval = 1;
+	}
 	return(retval);
 }
 
diff --git a/src/pacman/util.c b/src/pacman/util.c
index ed7669a..2cef54f 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -42,6 +42,34 @@
 /* pacman */
 #include "util.h"
 #include "conf.h"
+#include "callback.h"
+
+
+int trans_init(pmtranstype_t type, pmtransflag_t flags)
+{
+	if(alpm_trans_init(type, flags, cb_trans_evt,
+				cb_trans_conv, cb_trans_progress) == -1) {
+		/* TODO: error messages should be in the front end, not the back */
+		fprintf(stderr, _("error: failed to init transaction (%s)\n"),
+				alpm_strerrorlast());
+		if(pm_errno == PM_ERR_HANDLE_LOCK) {
+			fprintf(stderr, _("  if you're sure a package manager is not already\n"
+						"  running, you can remove %s\n"), alpm_option_get_lockfile());
+		}
+		return(-1);
+	}
+	return(0);
+}
+
+int trans_release()
+{
+	if(alpm_trans_release() == -1) {
+		fprintf(stderr, _("error: failed to release transaction (%s)\n"),
+				alpm_strerrorlast());
+		return(-1);
+	}
+	return(0);
+}
 
 int needs_transaction()
 {
diff --git a/src/pacman/util.h b/src/pacman/util.h
index f2facbf..722e4ab 100644
--- a/src/pacman/util.h
+++ b/src/pacman/util.h
@@ -36,6 +36,8 @@
 /* update speed for the fill_progress based functions */
 #define UPDATE_SPEED_SEC 0.2f
 
+int trans_init(pmtranstype_t type, pmtransflag_t flags);
+int trans_release();
 int needs_transaction();
 int getcols();
 int makepath(const char *path);
-- 
1.5.5.1

    