[pacman-dev] [PATCH] Convert backup list to new pmbackup_t type

Dan McGee dan at archlinux.org
Mon Jun 20 11:40:39 EDT 2011


This allows us to separate the name and hash elements in one place and
not scatter different parsing code all over the place, including both
the frontend and backend.

Signed-off-by: Dan McGee <dan at archlinux.org>
---

While the API changing season is still ongoing, although this one has pissed me
off for a while (WTF are we doing parsing the '\t' in the frontend and backend
multiple times?). Recommend starting with the alpm.h and backup.h changes to
get a quick overview, then reviewing the rest of the patch.

-Dan

 lib/libalpm/add.c        |   67 +++++++++++++++-----------------------
 lib/libalpm/alpm.h       |    6 +++
 lib/libalpm/backup.c     |   80 +++++++++++++++++++--------------------------
 lib/libalpm/backup.h     |    8 +++--
 lib/libalpm/be_local.c   |   12 ++++--
 lib/libalpm/be_package.c |    5 ++-
 lib/libalpm/package.c    |    7 +++-
 lib/libalpm/package.h    |    1 +
 lib/libalpm/remove.c     |   22 ++++++-------
 src/pacman/package.c     |   19 ++++-------
 10 files changed, 106 insertions(+), 121 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 7b394a5..fd8799b 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -138,7 +138,7 @@ static int extract_single_file(pmhandle_t *handle, struct archive *archive,
 	mode_t entrymode;
 	char filename[PATH_MAX]; /* the actual file we're extracting */
 	int needbackup = 0, notouch = 0;
-	char *hash_orig = NULL;
+	const char *hash_orig = NULL;
 	char *entryname_orig = NULL;
 	int errors = 0;
 
@@ -252,23 +252,26 @@ static int extract_single_file(pmhandle_t *handle, struct archive *archive,
 			if(alpm_list_find_str(handle->noupgrade, entryname)) {
 				notouch = 1;
 			} else {
+				pmbackup_t *backup;
 				/* go to the backup array and see if our conflict is there */
 				/* check newpkg first, so that adding backup files is retroactive */
-				if(alpm_list_find_str(alpm_pkg_get_backup(newpkg), entryname) != NULL) {
+				backup = _alpm_needbackup(entryname, alpm_pkg_get_backup(newpkg));
+				if(backup) {
 					needbackup = 1;
 				}
 
 				/* check oldpkg for a backup entry, store the hash if available */
 				if(oldpkg) {
-					hash_orig = _alpm_needbackup(entryname, alpm_pkg_get_backup(oldpkg));
-					if(hash_orig) {
+					backup = _alpm_needbackup(entryname, alpm_pkg_get_backup(oldpkg));
+					if(backup) {
+						hash_orig = backup->hash;
 						needbackup = 1;
 					}
 				}
 
 				/* if we force hash_orig to be non-NULL retroactive backup works */
 				if(needbackup && !hash_orig) {
-					STRDUP(hash_orig, "", RET_ERR(handle, PM_ERR_MEMORY, -1));
+					hash_orig = "";
 				}
 			}
 		}
@@ -290,7 +293,6 @@ static int extract_single_file(pmhandle_t *handle, struct archive *archive,
 		ret = perform_extraction(handle, archive, entry, checkfile, entryname_orig);
 		if(ret == 1) {
 			/* error */
-			FREE(hash_orig);
 			FREE(entryname_orig);
 			return 1;
 		}
@@ -298,24 +300,17 @@ static int extract_single_file(pmhandle_t *handle, struct archive *archive,
 		hash_local = alpm_compute_md5sum(filename);
 		hash_pkg = alpm_compute_md5sum(checkfile);
 
-		/* append the new md5 hash to it's respective entry
-		 * in newpkg's backup (it will be the new orginal) */
-		alpm_list_t *backups;
-		for(backups = alpm_pkg_get_backup(newpkg); backups;
-				backups = alpm_list_next(backups)) {
-			char *oldbackup = alpm_list_getdata(backups);
-			if(!oldbackup || strcmp(oldbackup, entryname_orig) != 0) {
+		/* update the md5 hash in newpkg's backup (it will be the new orginal) */
+		alpm_list_t *i;
+		for(i = alpm_pkg_get_backup(newpkg); i; i = i->next) {
+			pmbackup_t *backup = i->data;
+			char *newhash;
+			if(!backup->name || strcmp(backup->name, entryname_orig) != 0) {
 				continue;
 			}
-			char *backup = NULL;
-			/* length is tab char, null byte and MD5 (32 char) */
-			size_t backup_len = strlen(oldbackup) + 34;
-			MALLOC(backup, backup_len, RET_ERR(handle, PM_ERR_MEMORY, -1));
-
-			sprintf(backup, "%s\t%s", oldbackup, hash_pkg);
-			backup[backup_len-1] = '\0';
-			FREE(oldbackup);
-			backups->data = backup;
+			STRDUP(newhash, hash_pkg, RET_ERR(handle, PM_ERR_MEMORY, -1));
+			FREE(backup->hash);
+			backup->hash = newhash;
 		}
 
 		_alpm_log(handle, PM_LOG_DEBUG, "checking hashes for %s\n", entryname_orig);
@@ -409,7 +404,6 @@ static int extract_single_file(pmhandle_t *handle, struct archive *archive,
 
 		FREE(hash_local);
 		FREE(hash_pkg);
-		FREE(hash_orig);
 	} else {
 		int ret;
 
@@ -439,26 +433,17 @@ static int extract_single_file(pmhandle_t *handle, struct archive *archive,
 		}
 
 		/* calculate an hash if this is in newpkg's backup */
-		alpm_list_t *b;
-		for(b = alpm_pkg_get_backup(newpkg); b; b = b->next) {
-			char *backup = NULL, *hash = NULL;
-			char *oldbackup = alpm_list_getdata(b);
-			/* length is tab char, null byte and MD5 (32 char) */
-			size_t backup_len = strlen(oldbackup) + 34;
-
-			if(!oldbackup || strcmp(oldbackup, entryname_orig) != 0) {
+		alpm_list_t *i;
+		for(i = alpm_pkg_get_backup(newpkg); i; i = i->next) {
+			pmbackup_t *backup = i->data;
+			char *newhash;
+			if(!backup->name || strcmp(backup->name, entryname_orig) != 0) {
 				continue;
 			}
-			_alpm_log(handle, PM_LOG_DEBUG, "appending backup entry for %s\n", filename);
-
-			hash = alpm_compute_md5sum(filename);
-			MALLOC(backup, backup_len, RET_ERR(handle, PM_ERR_MEMORY, -1));
-
-			sprintf(backup, "%s\t%s", oldbackup, hash);
-			backup[backup_len-1] = '\0';
-			FREE(hash);
-			FREE(oldbackup);
-			b->data = backup;
+			_alpm_log(handle, PM_LOG_DEBUG, "appending backup entry for %s\n", entryname_orig);
+			newhash = alpm_compute_md5sum(filename);
+			FREE(backup->hash);
+			backup->hash = newhash;
 		}
 	}
 	FREE(entryname_orig);
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index d1faf7f..715e502 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -160,6 +160,12 @@ typedef struct _pmdelta_t {
 	off_t download_size;
 } pmdelta_t;
 
+/** Local package or package file backup entry */
+typedef struct _pmbackup_t {
+	char *name;
+	char *hash;
+} pmbackup_t;
+
 /*
  * Logging facilities
  */
diff --git a/lib/libalpm/backup.c b/lib/libalpm/backup.c
index 6431b28..20fb8a8 100644
--- a/lib/libalpm/backup.c
+++ b/lib/libalpm/backup.c
@@ -32,54 +32,33 @@
 #include "log.h"
 #include "util.h"
 
-/* split a backup string "file\thash" into two strings : file and hash */
-static int backup_split(const char *string, char **file, char **hash)
+/* split a backup string "file\thash" into the relevant components */
+int _alpm_split_backup(const char *string, pmbackup_t **backup)
 {
-	char *str = strdup(string);
-	char *ptr;
+	char *str, *ptr;
+
+	STRDUP(str, string, return -1);
 
 	/* tab delimiter */
 	ptr = strchr(str, '\t');
 	if(ptr == NULL) {
-		if(file) {
-			*file = str;
-		} else {
-			/* don't need our dup as the fname wasn't requested, so free it */
-			FREE(str);
-		}
+		(*backup)->name = str;
+		(*backup)->hash = NULL;
 		return 0;
 	}
 	*ptr = '\0';
 	ptr++;
 	/* now str points to the filename and ptr points to the hash */
-	if(file) {
-		*file = strdup(str);
-	}
-	if(hash) {
-		*hash = strdup(ptr);
-	}
+	STRDUP((*backup)->name, str, return -1);
+	STRDUP((*backup)->hash, ptr, return -1);
 	FREE(str);
-	return 1;
-}
-
-char *_alpm_backup_file(const char *string)
-{
-	char *file = NULL;
-	backup_split(string, &file, NULL);
-	return file;
-}
-
-char *_alpm_backup_hash(const char *string)
-{
-	char *hash = NULL;
-	backup_split(string, NULL, &hash);
-	return hash;
+	return 0;
 }
 
-/* Look for a filename in a pmpkg_t.backup list.  If we find it,
- * then we return the md5 hash (parsed from the same line)
+/* Look for a filename in a pmpkg_t.backup list. If we find it,
+ * then we return the full backup entry.
  */
-char *_alpm_needbackup(const char *file, const alpm_list_t *backup)
+pmbackup_t *_alpm_needbackup(const char *file, const alpm_list_t *backup)
 {
 	const alpm_list_t *lp;
 
@@ -89,23 +68,32 @@ char *_alpm_needbackup(const char *file, const alpm_list_t *backup)
 
 	/* run through the backup list and parse out the hash for our file */
 	for(lp = backup; lp; lp = lp->next) {
-		char *filename = NULL;
-		char *hash = NULL;
+		pmbackup_t *backup = lp->data;
 
-		/* no hash found */
-		if(!backup_split((char *)lp->data, &filename, &hash)) {
-			FREE(filename);
-			continue;
+		if(strcmp(file, backup->name) == 0) {
+			return backup;
 		}
-		if(strcmp(file, filename) == 0) {
-			FREE(filename);
-			return hash;
-		}
-		FREE(filename);
-		FREE(hash);
 	}
 
 	return NULL;
 }
 
+void _alpm_backup_free(pmbackup_t *backup)
+{
+	free(backup->name);
+	free(backup->hash);
+	free(backup);
+}
+
+pmbackup_t *_alpm_backup_dup(const pmbackup_t *backup)
+{
+	pmbackup_t *newbackup;
+	CALLOC(newbackup, 1, sizeof(pmbackup_t), return NULL);
+
+	STRDUP(newbackup->name, backup->name, return NULL);
+	STRDUP(newbackup->hash, backup->hash, return NULL);
+
+	return newbackup;
+}
+
 /* vim: set ts=2 sw=2 noet: */
diff --git a/lib/libalpm/backup.h b/lib/libalpm/backup.h
index 9475aa2..2f632d4 100644
--- a/lib/libalpm/backup.h
+++ b/lib/libalpm/backup.h
@@ -21,10 +21,12 @@
 #define _ALPM_BACKUP_H
 
 #include "alpm_list.h"
+#include "alpm.h"
 
-char *_alpm_backup_file(const char *string);
-char *_alpm_backup_hash(const char *string);
-char *_alpm_needbackup(const char *file, const alpm_list_t *backup);
+int _alpm_split_backup(const char *string, pmbackup_t **backup);
+pmbackup_t *_alpm_needbackup(const char *file, const alpm_list_t *backup);
+void _alpm_backup_free(pmbackup_t *backup);
+pmbackup_t *_alpm_backup_dup(const pmbackup_t *backup);
 
 #endif /* _ALPM_BACKUP_H */
 
diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index ee28db5..4b2a301 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -625,9 +625,12 @@ int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq)
 				}
 			} else if(strcmp(line, "%BACKUP%") == 0) {
 				while(fgets(line, sizeof(line), fp) && strlen(_alpm_strtrim(line))) {
-					char *linedup;
-					STRDUP(linedup, line, goto error);
-					info->backup = alpm_list_add(info->backup, linedup);
+					pmbackup_t *backup;
+					CALLOC(backup, 1, sizeof(pmbackup_t), goto error);
+					if(_alpm_split_backup(line, &backup)) {
+						goto error;
+					}
+					info->backup = alpm_list_add(info->backup, backup);
 				}
 			}
 		}
@@ -826,7 +829,8 @@ int _alpm_local_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq)
 		if(info->backup) {
 			fprintf(fp, "%%BACKUP%%\n");
 			for(lp = info->backup; lp; lp = lp->next) {
-				fprintf(fp, "%s\n", (char *)lp->data);
+				pmbackup_t *backup = lp->data;
+				fprintf(fp, "%s\t%s\n", backup->name, backup->hash);
 			}
 			fprintf(fp, "\n");
 		}
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
index 94df071..1807051 100644
--- a/lib/libalpm/be_package.c
+++ b/lib/libalpm/be_package.c
@@ -200,7 +200,10 @@ static int parse_descfile(pmhandle_t *handle, struct archive *a, pmpkg_t *newpkg
 			} else if(strcmp(key, "provides") == 0) {
 				newpkg->provides = alpm_list_add(newpkg->provides, strdup(ptr));
 			} else if(strcmp(key, "backup") == 0) {
-				newpkg->backup = alpm_list_add(newpkg->backup, strdup(ptr));
+				pmbackup_t *backup;
+				CALLOC(backup, 1, sizeof(pmbackup_t), return -1);
+				STRDUP(backup->name, ptr, return -1);
+				newpkg->backup = alpm_list_add(newpkg->backup, backup);
 			} else if(strcmp(key, "force") == 0) {
 				/* deprecated, skip it */
 			} else if(strcmp(key, "makepkgopt") == 0) {
diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index 0b0e974..e99d5bd 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -459,7 +459,9 @@ pmpkg_t *_alpm_pkg_dup(pmpkg_t *pkg)
 	newpkg->replaces   = alpm_list_strdup(pkg->replaces);
 	newpkg->groups     = alpm_list_strdup(pkg->groups);
 	newpkg->files      = alpm_list_strdup(pkg->files);
-	newpkg->backup     = alpm_list_strdup(pkg->backup);
+	for(i = pkg->backup; i; i = alpm_list_next(i)) {
+		newpkg->backup = alpm_list_add(newpkg->backup, _alpm_backup_dup(i->data));
+	}
 	for(i = pkg->depends; i; i = alpm_list_next(i)) {
 		newpkg->depends = alpm_list_add(newpkg->depends, _alpm_dep_dup(i->data));
 	}
@@ -507,7 +509,8 @@ void _alpm_pkg_free(pmpkg_t *pkg)
 	FREELIST(pkg->replaces);
 	FREELIST(pkg->groups);
 	FREELIST(pkg->files);
-	FREELIST(pkg->backup);
+	alpm_list_free_inner(pkg->backup, (alpm_list_fn_free)_alpm_backup_free);
+	alpm_list_free(pkg->backup);
 	alpm_list_free_inner(pkg->depends, (alpm_list_fn_free)_alpm_dep_free);
 	alpm_list_free(pkg->depends);
 	FREELIST(pkg->optdepends);
diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h
index bc5b267..d18020d 100644
--- a/lib/libalpm/package.h
+++ b/lib/libalpm/package.h
@@ -30,6 +30,7 @@
 #include <time.h> /* time_t */
 
 #include "alpm.h"
+#include "backup.h"
 #include "db.h"
 #include "signing.h"
 
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index 9f07501..b6a4c71 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -254,16 +254,14 @@ static void unlink_file(pmhandle_t *handle, pmpkg_t *info, const char *filename,
 		}
 	} else {
 		/* if the file needs backup and has been modified, back it up to .pacsave */
-		char *pkghash = _alpm_needbackup(filename, alpm_pkg_get_backup(info));
-		if(pkghash) {
+		pmbackup_t *backup = _alpm_needbackup(filename, alpm_pkg_get_backup(info));
+		if(backup) {
 			if(nosave) {
 				_alpm_log(handle, PM_LOG_DEBUG, "transaction is set to NOSAVE, not backing up '%s'\n", file);
-				FREE(pkghash);
 			} else {
 				char *filehash = alpm_compute_md5sum(file);
-				int cmp = filehash ? strcmp(filehash, pkghash) : 0;
+				int cmp = filehash ? strcmp(filehash, backup->hash) : 0;
 				FREE(filehash);
-				FREE(pkghash);
 				if(cmp != 0) {
 					char newpath[PATH_MAX];
 					snprintf(newpath, PATH_MAX, "%s.pacsave", file);
@@ -309,20 +307,20 @@ int _alpm_upgraderemove_package(pmhandle_t *handle,
 	/* old package backup list */
 	alpm_list_t *filelist = alpm_pkg_get_files(newpkg);
 	for(b = alpm_pkg_get_backup(newpkg); b; b = b->next) {
-		char *backup = _alpm_backup_file(b->data);
+		const pmbackup_t *backup = b->data;
 		/* safety check (fix the upgrade026 pactest) */
-		if(!alpm_list_find_str(filelist, backup)) {
-			FREE(backup);
+		if(!alpm_list_find_str(filelist, backup->name)) {
 			continue;
 		}
-		_alpm_log(handle, PM_LOG_DEBUG, "adding %s to the skip_remove array\n", backup);
-		skip_remove = alpm_list_add(skip_remove, backup);
+		_alpm_log(handle, PM_LOG_DEBUG, "adding %s to the skip_remove array\n",
+				backup->name);
+		skip_remove = alpm_list_add(skip_remove, strdup(backup->name));
 	}
 
 	for(lp = files; lp; lp = lp->next) {
 		if(!can_remove_file(handle, lp->data, skip_remove)) {
-			_alpm_log(handle, PM_LOG_DEBUG, "not removing package '%s', can't remove all files\n",
-					pkgname);
+			_alpm_log(handle, PM_LOG_DEBUG,
+					"not removing package '%s', can't remove all files\n", pkgname);
 			RET_ERR(handle, PM_ERR_PKG_CANT_REMOVE, -1);
 		}
 	}
diff --git a/src/pacman/package.c b/src/pacman/package.c
index 9cdb487..32156c5 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -151,12 +151,12 @@ void dump_pkg_full(pmpkg_t *pkg, enum pkg_from from, int extra)
 }
 
 static const char *get_backup_file_status(const char *root,
-		const char *filename, const char *expected_md5)
+		const pmbackup_t *backup)
 {
 	char path[PATH_MAX];
 	const char *ret;
 
-	snprintf(path, PATH_MAX, "%s%s", root, filename);
+	snprintf(path, PATH_MAX, "%s%s", root, backup->name);
 
 	/* if we find the file, calculate checksums, otherwise it is missing */
 	if(access(path, R_OK) == 0) {
@@ -169,7 +169,7 @@ static const char *get_backup_file_status(const char *root,
 		}
 
 		/* if checksums don't match, file has been modified */
-		if(strcmp(md5sum, expected_md5) != 0) {
+		if(strcmp(md5sum, backup->hash) != 0) {
 			ret = "MODIFIED";
 		} else {
 			ret = "UNMODIFIED";
@@ -200,18 +200,13 @@ void dump_pkg_backups(pmpkg_t *pkg)
 	if(alpm_pkg_get_backup(pkg)) {
 		/* package has backup files, so print them */
 		for(i = alpm_pkg_get_backup(pkg); i; i = alpm_list_next(i)) {
+			const pmbackup_t *backup = alpm_list_getdata(i);
 			const char *value;
-			char *str = strdup(alpm_list_getdata(i));
-			char *ptr = strchr(str, '\t');
-			if(ptr == NULL) {
-				free(str);
+			if(!backup->hash) {
 				continue;
 			}
-			*ptr = '\0';
-			ptr++;
-			value = get_backup_file_status(root, str, ptr);
-			printf("%s\t%s%s\n", value, root, str);
-			free(str);
+			value = get_backup_file_status(root, backup);
+			printf("%s\t%s%s\n", value, root, backup->name);
 		}
 	} else {
 		/* package had no backup files */
-- 
1.7.5.4



More information about the pacman-dev mailing list