[pacman-dev] [PATCH 2/2] Backup old database before updating and restore on failure

[Allan McRae]

If alpm_db_update() fails due to an invalid signature, then the system
is left with an unusable repo database.  Instead, backup the currently
working database before performing the update, and restore on error.

Note that the calls rename and unlink are not checked for errors. If these
fail, there is nothing to be done anyway.  It also allows for less complex
flow, as these function fail gracefully when passed NULL arguments.

Signed-off-by: Allan McRae <allan at archlinux.org>
---

That is a lot of teduim for adding ".bak" to the end of a string...

I'd appreciate more eyes on these changes.

 lib/libalpm/be_sync.c | 58 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 5502d92d..ae46c120 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -136,6 +136,7 @@ valid:
 	return 0;
 }
 
+
 /** Update a package database
  *
  * An update of the package database \a db will be attempted. Unless
@@ -173,14 +174,15 @@ valid:
  */
 int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 {
-	char *syncpath;
-	const char *dbext;
+	char *syncpath, *dbsig = NULL, *dbfilebak = NULL, *dbsigbak = NULL;
+	const char *dbext, *dbfile;
 	alpm_list_t *i;
 	int updated = 0;
 	int ret = -1;
 	mode_t oldmask;
 	alpm_handle_t *handle;
 	int siglevel;
+	size_t len;
 
 	/* Sanity checks */
 	ASSERT(db != NULL, return -1);
@@ -217,10 +219,39 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 
 	dbext = handle->dbext;
 
+	/* create backup of current database version */
+	dbfile = _alpm_db_path(db);
+	len = strlen(dbfile) + 5;
+	MALLOC(dbfilebak, len,
+		{
+			handle->pm_errno = ALPM_ERR_MEMORY;
+			ret = -1;
+			goto cleanup;
+		}
+	);
+	snprintf(dbfilebak, len, "%s.bak", dbfile);
+
+
+	dbsig = _alpm_sigpath(db->handle, dbfile);
+	len = strlen(dbsig) + 5;
+	MALLOC(dbsigbak, len,
+		{
+			handle->pm_errno = ALPM_ERR_MEMORY;
+			ret = -1;
+			goto cleanup;
+		}
+	);
+	snprintf(dbsigbak, len, "%s.bak", dbsig);
+
+	/* remove any old backup files to prevent potential sig mismatch */
+	unlink(dbfilebak);
+	unlink(dbsigbak);
+	_alpm_copyfile(dbfile, dbfilebak);
+	_alpm_copyfile(dbsig, dbsigbak);
+
 	for(i = db->servers; i; i = i->next) {
 		const char *server = i->data, *final_db_url = NULL;
 		struct dload_payload payload;
-		size_t len;
 		int sig_ret = 0;
 
 		memset(&payload, 0, sizeof(struct dload_payload));
@@ -247,17 +278,6 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 		updated = (updated || ret == 0);
 
 		if(ret != -1 && updated && (siglevel & ALPM_SIG_DATABASE)) {
-			/* an existing sig file is no good at this point */
-			char *sigpath = _alpm_sigpath(handle, _alpm_db_path(db));
-			if(!sigpath) {
-				/* pm_errno should be set */
-				ret = -1;
-				goto cleanup;
-			}
-			unlink(sigpath);
-			free(sigpath);
-
-
 			/* check if the final URL from internal downloader looks reasonable */
 			if(final_db_url != NULL) {
 				if(strlen(final_db_url) < 3
@@ -327,13 +347,23 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 
 cleanup:
 	if(ret == -1) {
+		rename(dbfilebak, dbfile);
+		rename(dbsigbak, dbsig);
+
 		/* pm_errno was set by the download code */
 		_alpm_log(handle, ALPM_LOG_DEBUG, "failed to sync db: %s\n",
 				alpm_strerror(handle->pm_errno));
 	} else {
+		unlink(dbfilebak);
+		unlink(dbsigbak);
+
 		handle->pm_errno = ALPM_ERR_OK;
 	}
 
+	free(dbfilebak);
+	free(dbsig);
+	free(dbsigbak);
+
 	_alpm_handle_unlock(handle);
 	free(syncpath);
 	umask(oldmask);
-- 
2.24.1