The previous implementation was overly complex with unnecessary checks and nested conditionals. By reordering the tests and changing them to all be checks for positive hash matches rather than non-matches, we can collapse several cases and make the process much more linear. This removes the need to set hash_orig = "" just to reach some of the checks and corrects a faulty assumption that files are equivalent when the hashing process fails.
Signed-off-by: Andrew Gregory <[email protected]> --- lib/libalpm/add.c | 117 ++++++++++++++++++++++++++---------------------------- 1 file changed, 57 insertions(+), 60 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 3ef81e3..9cbf767 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -273,8 +273,6 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, /* check newpkg first, so that adding backup files is retroactive */ backup = _alpm_needbackup(entryname, newpkg); if(backup) { - /* if we force hash_orig to be non-NULL retroactive backup works */ - hash_orig = ""; needbackup = 1; } @@ -332,81 +330,80 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, _alpm_log(handle, ALPM_LOG_DEBUG, "new: %s\n", hash_pkg); _alpm_log(handle, ALPM_LOG_DEBUG, "original: %s\n", hash_orig); - if(!oldpkg) { - if(hash_local && hash_pkg && strcmp(hash_local, hash_pkg) != 0) { - /* looks like we have a local file that has a different hash as the - * file in the package, move it to a .pacorig */ + if(hash_local && hash_pkg && strcmp(hash_local, hash_pkg) == 0) { + /* local and new files are the same, no sense in installing the file + * over itself, regardless of what the original file was */ + _alpm_log(handle, ALPM_LOG_DEBUG, + "action: leaving existing file in place\n"); + unlink(checkfile); + } else if(hash_orig && hash_pkg && strcmp(hash_orig, hash_pkg) == 0) { + /* original and new files are the same, leave the local version alone, + * including any user changes */ + _alpm_log(handle, ALPM_LOG_DEBUG, + "action: leaving existing file in place\n"); + unlink(checkfile); + } else if(hash_orig && hash_local && strcmp(hash_orig, hash_local) == 0) { + /* installed file has NOT been changed by user, + * update to the new version */ + _alpm_log(handle, ALPM_LOG_DEBUG, "action: installing new file: %s\n", + entryname_orig); + if(try_rename(handle, checkfile, filename)) { + errors++; + } + } else { + /* none of the three files matched another, unpack the new file alongside + * the local file */ + + if(oldpkg) { char *newpath; - size_t newlen = strlen(filename) + 9; + size_t newlen = strlen(filename) + strlen(".pacnew") + 1; + + _alpm_log(handle, ALPM_LOG_DEBUG, + "action: keeping current file and installing" + " new one with .pacnew ending\n"); + + MALLOC(newpath, newlen, + errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); + snprintf(newpath, newlen, "%s.pacnew", filename); + + if(try_rename(handle, checkfile, newpath)) { + errors++; + } else { + _alpm_log(handle, ALPM_LOG_WARNING, _("%s installed as %s\n"), + filename, newpath); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "warning: %s installed as %s\n", filename, newpath); + } + + free(newpath); + } else { + char *newpath; + size_t newlen = strlen(filename) + strlen(".pacorig") + 1; + + _alpm_log(handle, ALPM_LOG_DEBUG, + "action: saving existing file with a .pacorig ending" + " and installing a new one\n"); + MALLOC(newpath, newlen, errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); snprintf(newpath, newlen, "%s.pacorig", filename); /* move the existing file to the "pacorig" */ if(try_rename(handle, filename, newpath)) { - errors++; - errors++; + errors++; /* failed rename filename -> filename.pacorig */ + errors++; /* failed rename checkfile -> filename */ } else { /* rename the file we extracted to the real name */ if(try_rename(handle, checkfile, filename)) { errors++; } else { - _alpm_log(handle, ALPM_LOG_WARNING, _("%s saved as %s\n"), filename, newpath); + _alpm_log(handle, ALPM_LOG_WARNING, + _("%s saved as %s\n"), filename, newpath); alpm_logaction(handle, ALPM_CALLER_PREFIX, "warning: %s saved as %s\n", filename, newpath); } } - free(newpath); - } else { - /* local file is identical to pkg one, so just remove pkg one */ - unlink(checkfile); - } - } else if(hash_orig) { - /* the fun part */ - - if(hash_local && strcmp(hash_orig, hash_local) == 0) { - /* installed file has NOT been changed by user */ - if(hash_pkg && strcmp(hash_orig, hash_pkg) != 0) { - _alpm_log(handle, ALPM_LOG_DEBUG, "action: installing new file: %s\n", - entryname_orig); - if(try_rename(handle, checkfile, filename)) { - errors++; - } - } else { - /* no sense in installing the same file twice, install - * ONLY if the original and package hashes differ */ - _alpm_log(handle, ALPM_LOG_DEBUG, "action: leaving existing file in place\n"); - unlink(checkfile); - } - } else if(hash_pkg && strcmp(hash_orig, hash_pkg) == 0) { - /* originally installed file and new file are the same - this - * implies the case above failed - i.e. the file was changed by a - * user */ - _alpm_log(handle, ALPM_LOG_DEBUG, "action: leaving existing file in place\n"); - unlink(checkfile); - } else if(hash_local && hash_pkg && strcmp(hash_local, hash_pkg) == 0) { - /* this would be magical. The above two cases failed, but the - * user changes just so happened to make the new file exactly the - * same as the one in the package... skip it */ - _alpm_log(handle, ALPM_LOG_DEBUG, "action: leaving existing file in place\n"); - unlink(checkfile); - } else { - char *newpath; - size_t newlen = strlen(filename) + 8; - _alpm_log(handle, ALPM_LOG_DEBUG, "action: keeping current file and installing" - " new one with .pacnew ending\n"); - MALLOC(newpath, newlen, - errors++; handle->pm_errno = ALPM_ERR_MEMORY; goto needbackup_cleanup); - snprintf(newpath, newlen, "%s.pacnew", filename); - if(try_rename(handle, checkfile, newpath)) { - errors++; - } else { - _alpm_log(handle, ALPM_LOG_WARNING, _("%s installed as %s\n"), - filename, newpath); - alpm_logaction(handle, ALPM_CALLER_PREFIX, - "warning: %s installed as %s\n", filename, newpath); - } free(newpath); } } -- 1.8.2.3
