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@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