From 5cfa4ec47ed6240fecb1c2a9fd95de5bdc58a8b8 Mon Sep 17 00:00:00 2001 From: Andrew Gregory Date: Fri, 12 Apr 2013 20:37:56 -0400 Subject: alpm_filelist: remove resolved_path Signed-off-by: Andrew Gregory Signed-off-by: Allan McRae --- lib/libalpm/alpm.h | 3 +- lib/libalpm/conflict.c | 14 +-- lib/libalpm/filelist.c | 225 ++--------------------------------- lib/libalpm/filelist.h | 4 - lib/libalpm/package.c | 22 +--- lib/libalpm/remove.c | 5 - test/pacman/tests/fileconflict001.py | 2 + test/pacman/tests/fileconflict013.py | 2 - test/pacman/tests/fileconflict016.py | 2 + test/pacman/tests/fileconflict017.py | 2 + test/pacman/tests/fileconflict022.py | 2 + test/pacman/tests/fileconflict023.py | 2 - test/pacman/tests/fileconflict025.py | 2 - 13 files changed, 24 insertions(+), 263 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 806cd959..c6d97c59 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -217,7 +217,6 @@ typedef struct _alpm_file_t { typedef struct _alpm_filelist_t { size_t count; alpm_file_t *files; - char **resolved_path; } alpm_filelist_t; /** Local package or package file backup entry */ @@ -1042,7 +1041,7 @@ int alpm_pkg_set_reason(alpm_pkg_t *pkg, alpm_pkgreason_t reason); * @param path the path to search for in the package * @return a pointer to the matching file or NULL if not found */ -char *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path); +alpm_file_t *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path); /* * Signatures diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 041a04ef..896f48de 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -321,7 +321,6 @@ static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath, const char *root = handle->root; /* check directory is actually in package - used for subdirectory checks */ - _alpm_filelist_resolve(handle, alpm_pkg_get_files(pkg)); if(!alpm_filelist_contains(alpm_pkg_get_files(pkg), dirpath)) { _alpm_log(handle, ALPM_LOG_DEBUG, "directory %s not in package %s\n", dirpath, pkg->name); @@ -340,7 +339,6 @@ static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath, continue; } - _alpm_filelist_resolve(handle, alpm_pkg_get_files(i->data)); if(alpm_filelist_contains(alpm_pkg_get_files(i->data), dirpath)) { _alpm_log(handle, ALPM_LOG_DEBUG, "file %s also in package %s\n", dirpath, @@ -375,7 +373,6 @@ static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath, return 0; } } else { - _alpm_filelist_resolve(handle, alpm_pkg_get_files(pkg)); if(alpm_filelist_contains(alpm_pkg_get_files(pkg), path)) { continue; } else { @@ -417,11 +414,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, rootlen = strlen(handle->root); - /* make sure all files to be installed have been resolved */ - for(i = upgrade; i; i = i->next) { - _alpm_filelist_resolve(handle, alpm_pkg_get_files(i->data)); - } - /* TODO this whole function needs a huge change, which hopefully will * be possible with real transactions. Right now we only do half as much * here as we do when we actually extract files in add.c with our 12 @@ -491,7 +483,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, * be freed. */ if(dbpkg) { /* older ver of package currently installed */ - _alpm_filelist_resolve(handle, alpm_pkg_get_files(dbpkg)); tmpfiles = _alpm_filelist_difference(alpm_pkg_get_files(p1), alpm_pkg_get_files(dbpkg)); } else { @@ -499,7 +490,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_filelist_t *fl = alpm_pkg_get_files(p1); size_t filenum; for(filenum = 0; filenum < fl->count; filenum++) { - tmpfiles = alpm_list_add(tmpfiles, fl->resolved_path[filenum]); + tmpfiles = alpm_list_add(tmpfiles, fl->files[filenum].name); } } @@ -545,7 +536,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, /* Check remove list (will we remove the conflicting local file?) */ for(k = rem; k && !resolved_conflict; k = k->next) { alpm_pkg_t *rempkg = k->data; - _alpm_filelist_resolve(handle, alpm_pkg_get_files(rempkg)); if(rempkg && alpm_filelist_contains(alpm_pkg_get_files(rempkg), relative_path)) { _alpm_log(handle, ALPM_LOG_DEBUG, @@ -565,7 +555,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, localp2 = _alpm_db_get_pkgfromcache(handle->db_local, p2->name); /* localp2->files will be removed (target conflicts are handled by CHECK 1) */ - _alpm_filelist_resolve(handle, alpm_pkg_get_files(localp2)); if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), filestr)) { /* skip removal of file, but not add. this will prevent a second * package from removing the file when it was already installed @@ -612,7 +601,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_list_t *local_pkgs = _alpm_db_get_pkgcache(handle->db_local); int found = 0; for(k = local_pkgs; k && !found; k = k->next) { - _alpm_filelist_resolve(handle, alpm_pkg_get_files(k->data)); if(alpm_filelist_contains(alpm_pkg_get_files(k->data), filestr)) { found = 1; } diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 697dd23b..f8db9a33 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -25,199 +25,6 @@ #include "filelist.h" #include "util.h" -/** Helper function for comparing strings when sorting */ -static int _alpm_filelist_strcmp(const void *s1, const void *s2) -{ - return strcmp(*(char **)s1, *(char **)s2); -} - -/* TODO make sure callers check the return value so we can bail on errors. - * For now we soldier on as best we can, skipping paths that are too long to - * resolve and using the original filenames on memory errors. */ -/** - * @brief Resolves a symlink and its children. - * - * @attention Pre-condition: files must be sorted! - * - * @param files filelist to resolve - * @param i pointer to the index in files to start processing, will point to - * the last file processed on return - * @param path absolute path for the symlink being resolved - * @param root_len length of the root portion of path - * @param resolving is file \i in \files a symlink that needs to be resolved - * - * @return 0 on success, -1 on error - */ -int _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t *i, - char *path, size_t root_len, int resolving) -{ - char *causal_dir = NULL; /* symlink being resolved */ - char *filename_r = NULL; /* resolved filename */ - size_t causal_dir_len = 0, causal_dir_r_len = 0; - - if(resolving) { - /* deal with the symlink being resolved */ - MALLOC(filename_r, PATH_MAX, goto error); - causal_dir = files->files[*i].name; - causal_dir_len = strlen(causal_dir); - if(realpath(path, filename_r) == NULL) { - files->resolved_path[*i] = causal_dir; - FREE(filename_r); - return -1; - } - causal_dir_r_len = strlen(filename_r + root_len) + 1; - if(causal_dir_r_len >= PATH_MAX) { - files->resolved_path[*i] = causal_dir; - FREE(filename_r); - return -1; - } - /* remove root_r from filename_r */ - memmove(filename_r, filename_r + root_len, causal_dir_r_len); - filename_r[causal_dir_r_len - 1] = '/'; - filename_r[causal_dir_r_len] = '\0'; - STRDUP(files->resolved_path[*i], filename_r, goto error); - (*i)++; - } - - for(; *i < files->count; (*i)++) { - char *filename = files->files[*i].name; - size_t filename_len = strlen(filename); - size_t filename_r_len = filename_len; - struct stat sbuf; - int exists; - - if(resolving) { - if(filename_len < causal_dir_len || strncmp(filename, causal_dir, causal_dir_len) != 0) { - /* not inside causal_dir anymore */ - break; - } - - filename_r_len = filename_len + causal_dir_r_len - causal_dir_len; - if(filename_r_len >= PATH_MAX) { - /* resolved path is too long */ - files->resolved_path[*i] = filename; - continue; - } - - strcpy(filename_r + causal_dir_r_len, filename + causal_dir_len); - } - - /* deal with files and paths too long to resolve*/ - if(filename[filename_len - 1] != '/' || root_len + filename_r_len >= PATH_MAX) { - if(resolving) { - STRDUP(files->resolved_path[*i], filename_r, goto error); - } else { - files->resolved_path[*i] = filename; - } - continue; - } - - /* construct absolute path and stat() */ - strcpy(path + root_len, resolving ? filename_r : filename); - exists = !_alpm_lstat(path, &sbuf); - - /* deal with symlinks */ - if(exists && S_ISLNK(sbuf.st_mode)) { - _alpm_filelist_resolve_link(files, i, path, root_len, 1); - continue; - } - - /* deal with normal directories */ - if(resolving) { - STRDUP(files->resolved_path[*i], filename_r, goto error); - } else { - files->resolved_path[*i] = filename; - } - - /* deal with children of non-existent directories to reduce lstat() calls */ - if(!exists) { - for((*i)++; *i < files->count; (*i)++) { - char *f = files->files[*i].name; - size_t f_len = strlen(f); - size_t f_r_len; - - if(f_len < filename_len || strncmp(f, filename, filename_len) != 0) { - /* not inside the non-existent dir anymore */ - break; - } - - f_r_len = f_len + causal_dir_r_len - causal_dir_len; - if(resolving && f_r_len <= PATH_MAX) { - strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); - STRDUP(files->resolved_path[*i], filename_r, goto error); - } else { - files->resolved_path[*i] = f; - } - } - (*i)--; - } - } - (*i)--; - - FREE(filename_r); - - return 0; - -error: - FREE(filename_r); - /* out of memory, set remaining files to their original names */ - for(; *i < files->count; (*i)++) { - files->resolved_path[*i] = files->files[*i].name; - } - (*i)--; - return -1; -} - -/** - * @brief Takes a file list and resolves all directory paths according to the - * filesystem - * - * @attention Pre-condition: files must be sorted! - * - * @note A symlink and directory at the same path in two difference packages - * causes a conflict so the filepath can not change as packages get installed - * - * @param handle the context handle - * @param files list of files to resolve - * - * @return 0 on success, -1 on error - */ -int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) -{ - char path[PATH_MAX]; - size_t root_len, i = 0; - int ret = 0; - - if(!files || files->resolved_path) { - return 0; - } - - CALLOC(files->resolved_path, files->count, sizeof(char *), return -1); - - /* not much point in going on if we can't even resolve root */ - if(realpath(handle->root, path) == NULL){ - return -1; - } - root_len = strlen(path); - if(root_len + 1 >= PATH_MAX) { - return -1; - } - /* append '/' if root is not "/" */ - if(path[root_len - 1] != '/') { - path[root_len] = '/'; - root_len++; - path[root_len] = '\0'; - } - - ret = _alpm_filelist_resolve_link(files, &i, path, root_len, 0); - - qsort(files->resolved_path, files->count, sizeof(char *), - _alpm_filelist_strcmp); - - return ret; -} - - /* Returns the difference of the provided two lists of files. * Pre-condition: both lists are sorted! * When done, free the list but NOT the contained data. @@ -229,8 +36,8 @@ alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, size_t ctrA = 0, ctrB = 0; while(ctrA < filesA->count && ctrB < filesB->count) { - char *strA = filesA->resolved_path[ctrA]; - char *strB = filesB->resolved_path[ctrB]; + char *strA = filesA->files[ctrA].name; + char *strB = filesB->files[ctrB].name; int cmp = strcmp(strA, strB); if(cmp < 0) { @@ -247,7 +54,7 @@ alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, /* ensure we have completely emptied pA */ while(ctrA < filesA->count) { - ret = alpm_list_add(ret, filesA->resolved_path[ctrA]); + ret = alpm_list_add(ret, filesA->files[ctrA].name); ctrA++; } @@ -269,17 +76,17 @@ alpm_list_t *_alpm_filelist_intersection(alpm_filelist_t *filesA, char *strA, *strB; isdirA = 0; - strA = filesA->resolved_path[ctrA]; + strA = filesA->files[ctrA].name; if(strA[strlen(strA)-1] == '/') { isdirA = 1; - strA = strndup(filesA->resolved_path[ctrA], strlen(strA)-1); + strA = strndup(strA, strlen(strA)-1); } isdirB = 0; - strB = filesB->resolved_path[ctrB]; + strB = filesB->files[ctrB].name; if(strB[strlen(strB)-1] == '/') { isdirB = 1; - strB = strndup(filesB->resolved_path[ctrB], strlen(strB)-1); + strB = strndup(strB, strlen(strB)-1); } cmp = strcmp(strA, strB); @@ -297,7 +104,7 @@ alpm_list_t *_alpm_filelist_intersection(alpm_filelist_t *filesA, /* when not directories, item in both qualifies as an intersect */ if(! (isdirA && isdirB)) { - ret = alpm_list_add(ret, filesA->resolved_path[ctrA]); + ret = alpm_list_add(ret, filesA->files[ctrA].name); } ctrA++; ctrB++; @@ -323,11 +130,10 @@ int _alpm_files_cmp(const void *f1, const void *f2) return strcmp(file1->name, file2->name); } - -char SYMEXPORT *alpm_filelist_contains(alpm_filelist_t *filelist, +alpm_file_t SYMEXPORT *alpm_filelist_contains(alpm_filelist_t *filelist, const char *path) { - alpm_file_t key, *match; + alpm_file_t key; if(!filelist) { return NULL; @@ -335,17 +141,8 @@ char SYMEXPORT *alpm_filelist_contains(alpm_filelist_t *filelist, key.name = (char *)path; - match = bsearch(&key, filelist->files, filelist->count, + return bsearch(&key, filelist->files, filelist->count, sizeof(alpm_file_t), _alpm_files_cmp); - - if(match) { - return match->name; - } else if(filelist->resolved_path) { - return bsearch(&path, filelist->resolved_path, filelist->count, - sizeof(char *), _alpm_filelist_strcmp); - } else { - return NULL; - } } /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/filelist.h b/lib/libalpm/filelist.h index bfab4165..66501f36 100644 --- a/lib/libalpm/filelist.h +++ b/lib/libalpm/filelist.h @@ -21,10 +21,6 @@ #include "alpm.h" -int _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t *i, - char *path, size_t root_len, int resolving); -int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files); - alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, alpm_filelist_t *filesB); diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 098c8677..cfdbb3f8 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -606,9 +606,6 @@ int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr) } } newpkg->files.count = pkg->files.count; - /* deliberately do not copy resolved_path as this is only used - * during conflict checking and the sorting of list does not readily - * allow keeping its efficient memory usage when copying */ } /* internal */ @@ -657,22 +654,9 @@ void _alpm_pkg_free(alpm_pkg_t *pkg) free_deplist(pkg->replaces); FREELIST(pkg->groups); if(pkg->files.count) { - size_t i, j, k; - if(pkg->files.resolved_path) { - for(i = 0, j = 0; i < pkg->files.count; i++) { - for(k = j; k <= pkg->files.count; k++) { - if(pkg->files.resolved_path[i] == pkg->files.files[k].name) { - pkg->files.files[k].name = NULL; - j = k + 1; - break; - } - } - free(pkg->files.resolved_path[i]); - } - free(pkg->files.resolved_path); - } - for(j = 0; j < pkg->files.count; j++) { - FREE(pkg->files.files[j].name); + size_t i; + for(i = 0; i < pkg->files.count; i++) { + FREE(pkg->files.files[i].name); } free(pkg->files.files); } diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index d0cd613c..652aa501 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -462,7 +462,6 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, "keeping directory %s (could not count files)\n", file); } else if(newpkg && alpm_filelist_contains(alpm_pkg_get_files(newpkg), fileobj->name)) { - /* newpkg's filelist should have already been resolved by the caller */ _alpm_log(handle, ALPM_LOG_DEBUG, "keeping directory %s (in new package)\n", file); } else if(dir_is_mountpoint(handle, file, &buf)) { @@ -484,9 +483,6 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, continue; } filelist = alpm_pkg_get_files(local_pkg); - /* This is too slow and only covers a rare case - Disable for now... */ - /* _alpm_filelist_resolve(handle, filelist); */ if(alpm_filelist_contains(filelist, fileobj->name)) { _alpm_log(handle, ALPM_LOG_DEBUG, "keeping directory %s (owned by %s)\n", file, local_pkg->name); @@ -584,7 +580,6 @@ static int remove_package_files(alpm_handle_t *handle, * so this removal operation doesn't kill them */ /* old package backup list */ newfiles = alpm_pkg_get_files(newpkg); - _alpm_filelist_resolve(handle, newfiles); for(b = alpm_pkg_get_backup(newpkg); b; b = b->next) { const alpm_backup_t *backup = b->data; /* safety check (fix the upgrade026 pactest) */ diff --git a/test/pacman/tests/fileconflict001.py b/test/pacman/tests/fileconflict001.py index b1ad5e18..e1371f88 100644 --- a/test/pacman/tests/fileconflict001.py +++ b/test/pacman/tests/fileconflict001.py @@ -25,3 +25,5 @@ self.addrule("!PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2") self.addrule("FILE_EXIST=dir/realdir/realfile") self.addrule("!FILE_EXIST=dir/realdir/file") + +self.expectfailure = True diff --git a/test/pacman/tests/fileconflict013.py b/test/pacman/tests/fileconflict013.py index 5c3a43b1..aacb730a 100644 --- a/test/pacman/tests/fileconflict013.py +++ b/test/pacman/tests/fileconflict013.py @@ -19,5 +19,3 @@ self.args = "-Su" self.addrule("PACMAN_RETCODE=1") self.addrule("PKG_VERSION=pkg1|1.0-1") - -self.expectfailure = True diff --git a/test/pacman/tests/fileconflict016.py b/test/pacman/tests/fileconflict016.py index 86ddd720..dbcf7085 100644 --- a/test/pacman/tests/fileconflict016.py +++ b/test/pacman/tests/fileconflict016.py @@ -22,3 +22,5 @@ self.args = "-S pkg1 pkg2" self.addrule("PACMAN_RETCODE=1") self.addrule("!PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2") + +self.expectfailure = True diff --git a/test/pacman/tests/fileconflict017.py b/test/pacman/tests/fileconflict017.py index 3855a938..4b91dc65 100644 --- a/test/pacman/tests/fileconflict017.py +++ b/test/pacman/tests/fileconflict017.py @@ -24,3 +24,5 @@ self.args = "-S pkg1 pkg2" self.addrule("PACMAN_RETCODE=1") self.addrule("!PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2") + +self.expectfailure = True diff --git a/test/pacman/tests/fileconflict022.py b/test/pacman/tests/fileconflict022.py index 5ff0b537..b3920dfd 100644 --- a/test/pacman/tests/fileconflict022.py +++ b/test/pacman/tests/fileconflict022.py @@ -16,3 +16,5 @@ self.args = "-S %s %s" % (sp1.name, sp2.name) self.addrule("PACMAN_RETCODE=1") self.addrule("!PKG_EXIST=foo") self.addrule("!PKG_EXIST=bar") + +self.expectfailure = True diff --git a/test/pacman/tests/fileconflict023.py b/test/pacman/tests/fileconflict023.py index 9685c0dd..1a7eaaf7 100644 --- a/test/pacman/tests/fileconflict023.py +++ b/test/pacman/tests/fileconflict023.py @@ -17,5 +17,3 @@ self.args = "-S %s --ask=4" % sp1.name self.addrule("PACMAN_RETCODE=1") self.addrule("PKG_EXIST=foo") self.addrule("!PKG_EXIST=bar") - -self.expectfailure = True diff --git a/test/pacman/tests/fileconflict025.py b/test/pacman/tests/fileconflict025.py index 195badbc..263c81d4 100644 --- a/test/pacman/tests/fileconflict025.py +++ b/test/pacman/tests/fileconflict025.py @@ -16,5 +16,3 @@ self.args = "-S %s --ask=4" % sp1.name self.addrule("PACMAN_RETCODE=1") self.addrule("PKG_EXIST=foo") self.addrule("!PKG_EXIST=bar") - -self.expectfailure = True -- cgit v1.2.3-70-g09d2