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 <[email protected]>
---

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


Reply via email to