Signed-off-by: Andrew Gregory --- 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 ccbdd1c..2cdac0c 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 */ @@ -1043,7 +1042,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 18e29a8..5e2fa1e 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, @@ -563,7 +553,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_pkg_t *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 @@ -610,7 +599,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 697dd23..f8db9a3 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 bfab416..66501f3 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 098c867..cfdbb3f 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 d0cd613..652aa50 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 b1ad5e1..e1371f8 100644 --- a/test/pacman/tests/fileconflict001.py +++ b/test/pacman/tests/fileconflict001.py @@ -25,3 +25,5 @@ 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 5c3a43b..aacb730 100644 --- a/test/pacman/tests/fileconflict013.py +++ b/test/pacman/tests/fileconflict013.py @@ -19,5 +19,3 @@ 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 86ddd72..dbcf708 100644 --- a/test/pacman/tests/fileconflict016.py +++ b/test/pacman/tests/fileconflict016.py @@ -22,3 +22,5 @@ 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 3855a93..4b91dc6 100644 --- a/test/pacman/tests/fileconflict017.py +++ b/test/pacman/tests/fileconflict017.py @@ -24,3 +24,5 @@ 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 5ff0b53..b3920df 100644 --- a/test/pacman/tests/fileconflict022.py +++ b/test/pacman/tests/fileconflict022.py @@ -16,3 +16,5 @@ 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 9685c0d..1a7eaaf 100644 --- a/test/pacman/tests/fileconflict023.py +++ b/test/pacman/tests/fileconflict023.py @@ -17,5 +17,3 @@ 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 195badb..263c81d 100644 --- a/test/pacman/tests/fileconflict025.py +++ b/test/pacman/tests/fileconflict025.py @@ -16,5 +16,3 @@ self.addrule("PACMAN_RETCODE=1") self.addrule("PKG_EXIST=foo") self.addrule("!PKG_EXIST=bar") - -self.expectfailure = True -- 1.8.2.1

In the event of a symlink<->dir transition, the paths for the incoming package need to be resolved to match the filelist for installed packages. Note: this only enables the symlink<->dir transition itself. Any installed packages with invalid file lists containing symlinks are not detected. Signed-off-by: Andrew Gregory --- lib/libalpm/conflict.c | 42 ++++++++++++++++------------------- src/common/util-common.c | 43 ++++++++++++++++++++++++++++++++++++ src/common/util-common.h | 1 + test/pacman/tests/fileconflict007.py | 2 -- test/pacman/tests/sync701.py | 2 -- test/pacman/tests/sync702.py | 2 -- 6 files changed, 63 insertions(+), 29 deletions(-) diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 518a2c6..08c1cbe 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -501,7 +501,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, /* have we acted on this conflict? */ int resolved_conflict = 0; struct stat lsbuf; - char path[PATH_MAX]; + char path[PATH_MAX], resolved_path[PATH_MAX]; size_t pathlen; pathlen = snprintf(path, PATH_MAX, "%s%s", handle->root, filestr); @@ -513,7 +513,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, _alpm_log(handle, ALPM_LOG_DEBUG, "checking possible conflict: %s\n", path); - if(filestr[strlen(filestr) - 1] == '/') { + if(path[pathlen - 1] == '/') { if(S_ISDIR(lsbuf.st_mode)) { _alpm_log(handle, ALPM_LOG_DEBUG, "file is a directory, not a conflict\n"); continue; @@ -524,7 +524,19 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, path[pathlen - 1] = '\0'; } - relative_path = path + rootlen; + if(lrealpath(path, resolved_path) + && strncmp(resolved_path, handle->root, rootlen) == 0) { + relative_path = resolved_path + rootlen; + } else { + relative_path = path + rootlen; + } + + if(!resolved_conflict && dbpkg + && alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_path)) { + _alpm_log(handle, ALPM_LOG_DEBUG, + "package contained the resolved realpath\n"); + resolved_conflict = 1; + } /* Check remove list (will we remove the conflicting local file?) */ for(k = rem; k && !resolved_conflict; k = k->next) { @@ -546,12 +558,12 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_pkg_t *localp2 = _alpm_db_get_pkgfromcache(handle->db_local, p2->name); /* localp2->files will be removed (target conflicts are handled by CHECK 1) */ - if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), filestr)) { + if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), relative_path)) { /* skip removal of file, but not add. this will prevent a second * package from removing the file when it was already installed * by its new owner (whether the file is in backup array or not */ handle->trans->skip_remove = - alpm_list_add(handle->trans->skip_remove, strdup(filestr)); + alpm_list_add(handle->trans->skip_remove, strdup(relative_path)); _alpm_log(handle, ALPM_LOG_DEBUG, "file changed packages, adding to remove skiplist\n"); resolved_conflict = 1; @@ -560,8 +572,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, /* check if all files of the dir belong to the installed pkg */ if(!resolved_conflict && S_ISDIR(lsbuf.st_mode) && dbpkg) { - char *dir = malloc(strlen(filestr) + 2); - sprintf(dir, "%s/", filestr); + char *dir = malloc(strlen(relative_path) + 2); + sprintf(dir, "%s/", relative_path); if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), dir)) { _alpm_log(handle, ALPM_LOG_DEBUG, "checking if all files in %s belong to %s\n", @@ -571,22 +583,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, free(dir); } - /* check if a component of the filepath was a link. canonicalize the path - * and look for it in the old package. note that the actual file under - * consideration cannot itself be a link, as it might be unowned- path - * components can be safely checked as all directories are "unowned". */ - if(!resolved_conflict && dbpkg && !S_ISLNK(lsbuf.st_mode)) { - char rpath[PATH_MAX]; - if(realpath(path, rpath)) { - const char *relative_rpath = rpath + rootlen; - if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_rpath)) { - _alpm_log(handle, ALPM_LOG_DEBUG, - "package contained the resolved realpath\n"); - resolved_conflict = 1; - } - } - } - /* is the file unowned and in the backup list of the new package? */ if(!resolved_conflict && _alpm_needbackup(filestr, p1)) { alpm_list_t *local_pkgs = _alpm_db_get_pkgcache(handle->db_local); diff --git a/src/common/util-common.c b/src/common/util-common.c index 7145e49..c2422e0 100644 --- a/src/common/util-common.c +++ b/src/common/util-common.c @@ -73,6 +73,49 @@ char *mdirname(const char *path) return strdup("."); } +/** Resolve the canonicalized absolute path of a symlink. + * @param path path to resolve + * @param resolved_path destination for the resolved path, will be malloc'd if + * NULL + * @return the resolved path + */ +char *lrealpath(const char *path, char *resolved_path) +{ + const char *bname = mbasename(path); + char *rpath = NULL, *dname = NULL; + int success = 0; + + if(strcmp(path, ".") == 0 || strcmp(path, "..") == 0) { + /* the entire path needs to be resolved */ + return realpath(path, resolved_path); + } + + if(!(dname = mdirname(path))) { + goto cleanup; + } + if(!(rpath = realpath(dname, NULL))) { + goto cleanup; + } + if(!resolved_path) { + if(!(resolved_path = malloc(strlen(rpath) + strlen(bname) + 2))) { + goto cleanup; + } + } + + strcpy(resolved_path, rpath); + if(resolved_path[strlen(resolved_path) - 1] != '/') { + strcat(resolved_path, "/"); + } + strcat(resolved_path, bname); + success = 1; + +cleanup: + free(dname); + free(rpath); + + return (success ? resolved_path : NULL); +} + #ifndef HAVE_STRNDUP /* A quick and dirty implementation derived from glibc */ /** Determines the length of a fixed-size string. diff --git a/src/common/util-common.h b/src/common/util-common.h index 04d4e9d..b3a4df1 100644 --- a/src/common/util-common.h +++ b/src/common/util-common.h @@ -22,6 +22,7 @@ const char *mbasename(const char *path); char *mdirname(const char *path); +char *lrealpath(const char *path, char *resolved_path); #ifndef HAVE_STRNDUP char *strndup(const char *s, size_t n); diff --git a/test/pacman/tests/fileconflict007.py b/test/pacman/tests/fileconflict007.py index b61ddb4..7fe65ed 100644 --- a/test/pacman/tests/fileconflict007.py +++ b/test/pacman/tests/fileconflict007.py @@ -16,5 +16,3 @@ self.addrule("PKG_EXIST=pkg") self.addrule("PKG_VERSION=pkg|1.0-2") self.addrule("FILE_TYPE=dir/symdir/|dir") - -self.expectfailure = True diff --git a/test/pacman/tests/sync701.py b/test/pacman/tests/sync701.py index 912c794..590845f 100644 --- a/test/pacman/tests/sync701.py +++ b/test/pacman/tests/sync701.py @@ -19,5 +19,3 @@ self.addrule("PKG_VERSION=pkg1|1.0-2") self.addrule("PKG_EXIST=pkg2") self.addrule("FILE_TYPE=lib|dir") - -self.expectfailure = True diff --git a/test/pacman/tests/sync702.py b/test/pacman/tests/sync702.py index 8f4c0ad..c3e2320 100644 --- a/test/pacman/tests/sync702.py +++ b/test/pacman/tests/sync702.py @@ -19,5 +19,3 @@ self.addrule("PKG_VERSION=pkg2|1.0-2") self.addrule("PKG_EXIST=pkg1") self.addrule("FILE_TYPE=lib|dir") - -self.expectfailure = True -- 1.8.2.1

We always want to work with the package file itself, not its target if it's a symlink. Signed-off-by: Andrew Gregory --- lib/libalpm/remove.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 652aa50..0b4f80c 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -442,11 +442,7 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, return 1; } - /* we want to do a lstat here, and not a _alpm_lstat. - * if a directory in the package is actually a directory symlink on the - * filesystem, we want to work with the linked directory instead of the - * actual symlink */ - if(lstat(file, &buf)) { + if(_alpm_lstat(file, &buf)) { _alpm_log(handle, ALPM_LOG_DEBUG, "file %s does not exist\n", file); return 1; } -- 1.8.2.1

Packages removed due to conflicts are always removed at the beginning of the transaction and as such can be included in the check for whether all owners of a directory will be removed in a transaction. Installed versions of packages being upgraded, other than the one with the conflict, cannot be used because our transaction ordering is not intelligent enough to ensure that they are removed prior to the installation of the conflicted package. Also, return false from dir_belongsto_pkgs on errors. Previously, we simply continued which could return true even if we were unable to actually establish that the package owned the entire tree. Signed-off-by: Andrew Gregory --- lib/libalpm/conflict.c | 133 +++++++++++++++++------------------ test/pacman/tests/fileconflict030.py | 17 +++++ 2 files changed, 82 insertions(+), 68 deletions(-) create mode 100644 test/pacman/tests/fileconflict030.py diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 08c1cbe..35ab6e8 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -299,94 +299,73 @@ void _alpm_fileconflict_free(alpm_fileconflict_t *conflict) } /** - * @brief Recursively checks if a package owns all subdirectories and files in - * a directory. + * @brief Recursively checks if a set of packages own all subdirectories and + * files in a directory. * * @param handle the context handle * @param dirpath path of the directory to check - * @param pkg package being checked against + * @param pkgs packages being checked against * - * @return 1 if a package owns all subdirectories and files or a directory - * cannot be opened, 0 otherwise + * @return 1 if a package owns all subdirectories and files, 0 otherwise */ -static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath, - alpm_pkg_t *pkg) +static int dir_belongsto_pkgs(alpm_handle_t *handle, const char *dirpath, + alpm_list_t *pkgs) { - alpm_list_t *i; - struct stat sbuf; - char path[PATH_MAX]; - char abspath[PATH_MAX]; + char path[PATH_MAX], full_path[PATH_MAX]; DIR *dir; struct dirent *ent = NULL; - const char *root = handle->root; - - /* check directory is actually in package - used for subdirectory checks */ - 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); - return 0; - } - - /* TODO: this is an overly strict check but currently pacman will not - * overwrite a directory with a file (case 10/11 in add.c). Adjusting that - * is not simple as even if the directory is being unowned by a conflicting - * package, pacman does not sort this to ensure all required directory - * "removals" happen before installation of file/symlink */ - - /* check that no other _installed_ package owns the directory */ - for(i = _alpm_db_get_pkgcache(handle->db_local); i; i = i->next) { - if(pkg == i->data) { - continue; - } - - 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, - ((alpm_pkg_t*)i->data)->name); - return 0; - } - } - /* check all files in directory are owned by the package */ - snprintf(abspath, PATH_MAX, "%s%s", root, dirpath); - dir = opendir(abspath); + snprintf(full_path, PATH_MAX, "%s%s", handle->root, dirpath); + dir = opendir(full_path); if(dir == NULL) { - return 1; + return 0; } while((ent = readdir(dir)) != NULL) { const char *name = ent->d_name; + int owned = 0; + alpm_list_t *i; + struct stat sbuf; if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) { continue; } + snprintf(path, PATH_MAX, "%s%s", dirpath, name); - snprintf(abspath, PATH_MAX, "%s%s", root, path); - if(stat(abspath, &sbuf) != 0) { - continue; - } - if(S_ISDIR(sbuf.st_mode)) { - if(dir_belongsto_pkg(handle, path, pkg)) { - continue; - } else { - closedir(dir); - return 0; - } - } else { - if(alpm_filelist_contains(alpm_pkg_get_files(pkg), path)) { - continue; - } else { - closedir(dir); - _alpm_log(handle, ALPM_LOG_DEBUG, - "unowned file %s found in directory\n", path); - return 0; + snprintf(full_path, PATH_MAX, "%s%s", handle->root, path); + + for(i = pkgs; i && !owned; i = i->next) { + if(alpm_filelist_contains(alpm_pkg_get_files(i->data), path)) { + owned = 1; } } + + if(owned && stat(full_path, &sbuf) != 0 && S_ISDIR(sbuf.st_mode)) { + owned = dir_belongsto_pkgs(handle, path, pkgs); + } + + if(!owned) { + closedir(dir); + _alpm_log(handle, ALPM_LOG_DEBUG, + "unowned file %s found in directory\n", path); + return 0; + } } closedir(dir); return 1; } +static alpm_list_t *alpm_db_find_file_owners(alpm_db_t* db, const char *path) +{ + alpm_list_t *i, *owners = NULL; + for(i = alpm_db_get_pkgcache(db); i; i = i->next) { + if(alpm_filelist_contains(alpm_pkg_get_files(i->data), path)) { + owners = alpm_list_add(owners, i->data); + } + } + return owners; +} + /** * @brief Find file conflicts that may occur during the transaction. * @@ -571,14 +550,32 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, } /* check if all files of the dir belong to the installed pkg */ - if(!resolved_conflict && S_ISDIR(lsbuf.st_mode) && dbpkg) { + if(!resolved_conflict && S_ISDIR(lsbuf.st_mode)) { + alpm_list_t *owners; char *dir = malloc(strlen(relative_path) + 2); sprintf(dir, "%s/", relative_path); - if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), dir)) { - _alpm_log(handle, ALPM_LOG_DEBUG, - "checking if all files in %s belong to %s\n", - dir, dbpkg->name); - resolved_conflict = dir_belongsto_pkg(handle, dir, dbpkg); + + owners = alpm_db_find_file_owners(handle->db_local, dir); + if(owners) { + alpm_list_t *pkgs = NULL, *diff; + + if(dbpkg) { + pkgs = alpm_list_add(pkgs, dbpkg); + } + pkgs = alpm_list_join(pkgs, alpm_list_copy(rem)); + if((diff = alpm_list_diff(owners, pkgs, _alpm_pkg_cmp))) { + /* dir is owned by files we aren't removing */ + /* TODO: with better commit ordering, we may be able to check + * against upgrades as well */ + alpm_list_free(diff); + } else { + _alpm_log(handle, ALPM_LOG_DEBUG, + "checking if all files in %s belong to removed packages\n", + dir); + resolved_conflict = dir_belongsto_pkgs(handle, dir, owners); + } + alpm_list_free(pkgs); + alpm_list_free(owners); } free(dir); } diff --git a/test/pacman/tests/fileconflict030.py b/test/pacman/tests/fileconflict030.py new file mode 100644 index 0000000..1de7781 --- /dev/null +++ b/test/pacman/tests/fileconflict030.py @@ -0,0 +1,17 @@ +self.description = "Dir->file transition filesystem conflict resolved by removal" + +lp1 = pmpkg("foo") +lp1.files = ["foo/"] +self.addpkg2db("local", lp1) + +sp1 = pmpkg("bar") +sp1.conflicts = ["foo"] +sp1.files = ["foo"] +self.addpkg2db("sync", sp1) + +self.args = "-S %s --ask=4" % sp1.name + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PKG_EXIST=bar") +self.addrule("!PKG_EXIST=foo") +self.addrule("FILE_EXIST=foo") -- 1.8.2.1

On 27/04/13 10:00, Andrew Gregory wrote:

In the event of a symlink<->dir transition, the paths for the incoming package need to be resolved to match the filelist for installed packages.

Can you explain this to me? Every time I think I understand, I don't... e.g. outdated local package: /usr/foo -> /usr/bar/ /usr/bar/baz update sync package: /usr/foo/baz So /usr/foo/baz is a new file so it is tested for presence in the filesystem. But /usr/foo/baz "exists" on the filesystem as /usr/bar/baz.

From what I understand, you resolve the package file "/usr/foo/baz" to "/usr/bar/baz" and note that it is in the old package so not a conflict. Which feels wrong to me...

Can we do something like testing whether the directory of the "conflicting" file and the resolved path of that directory are different? If not, then we must be dealing with a case of a filesystem symlink being replaced with a package directory (otherwise the package directory would have conflicted with the filesystem symlink earlier).

Note: this only enables the symlink<->dir transition itself. Any installed packages with invalid file lists containing symlinks are not detected.

Signed-off-by: Andrew Gregory --- lib/libalpm/conflict.c | 42 ++++++++++++++++------------------- src/common/util-common.c | 43 ++++++++++++++++++++++++++++++++++++ src/common/util-common.h | 1 + test/pacman/tests/fileconflict007.py | 2 -- test/pacman/tests/sync701.py | 2 -- test/pacman/tests/sync702.py | 2 -- 6 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 518a2c6..08c1cbe 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -501,7 +501,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, /* have we acted on this conflict? */ int resolved_conflict = 0; struct stat lsbuf; - char path[PATH_MAX]; + char path[PATH_MAX], resolved_path[PATH_MAX]; size_t pathlen;

pathlen = snprintf(path, PATH_MAX, "%s%s", handle->root, filestr); @@ -513,7 +513,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,

_alpm_log(handle, ALPM_LOG_DEBUG, "checking possible conflict: %s\n", path);

- if(filestr[strlen(filestr) - 1] == '/') { + if(path[pathlen - 1] == '/') { if(S_ISDIR(lsbuf.st_mode)) { _alpm_log(handle, ALPM_LOG_DEBUG, "file is a directory, not a conflict\n"); continue; @@ -524,7 +524,19 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, path[pathlen - 1] = '\0'; }

- relative_path = path + rootlen; + if(lrealpath(path, resolved_path) + && strncmp(resolved_path, handle->root, rootlen) == 0) { + relative_path = resolved_path + rootlen; + } else { + relative_path = path + rootlen; + } + + if(!resolved_conflict && dbpkg + && alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_path)) { + _alpm_log(handle, ALPM_LOG_DEBUG, + "package contained the resolved realpath\n"); + resolved_conflict = 1; + }

/* Check remove list (will we remove the conflicting local file?) */ for(k = rem; k && !resolved_conflict; k = k->next) { @@ -546,12 +558,12 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_pkg_t *localp2 = _alpm_db_get_pkgfromcache(handle->db_local, p2->name);

/* localp2->files will be removed (target conflicts are handled by CHECK 1) */ - if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), filestr)) { + if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), relative_path)) { /* skip removal of file, but not add. this will prevent a second * package from removing the file when it was already installed * by its new owner (whether the file is in backup array or not */ handle->trans->skip_remove = - alpm_list_add(handle->trans->skip_remove, strdup(filestr)); + alpm_list_add(handle->trans->skip_remove, strdup(relative_path)); _alpm_log(handle, ALPM_LOG_DEBUG, "file changed packages, adding to remove skiplist\n"); resolved_conflict = 1; @@ -560,8 +572,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,

/* check if all files of the dir belong to the installed pkg */ if(!resolved_conflict && S_ISDIR(lsbuf.st_mode) && dbpkg) { - char *dir = malloc(strlen(filestr) + 2); - sprintf(dir, "%s/", filestr); + char *dir = malloc(strlen(relative_path) + 2); + sprintf(dir, "%s/", relative_path); if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), dir)) { _alpm_log(handle, ALPM_LOG_DEBUG, "checking if all files in %s belong to %s\n", @@ -571,22 +583,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, free(dir); }

- /* check if a component of the filepath was a link. canonicalize the path - * and look for it in the old package. note that the actual file under - * consideration cannot itself be a link, as it might be unowned- path - * components can be safely checked as all directories are "unowned". */ - if(!resolved_conflict && dbpkg && !S_ISLNK(lsbuf.st_mode)) { - char rpath[PATH_MAX]; - if(realpath(path, rpath)) { - const char *relative_rpath = rpath + rootlen; - if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_rpath)) { - _alpm_log(handle, ALPM_LOG_DEBUG, - "package contained the resolved realpath\n"); - resolved_conflict = 1; - } - } - } - /* is the file unowned and in the backup list of the new package? */ if(!resolved_conflict && _alpm_needbackup(filestr, p1)) { alpm_list_t *local_pkgs = _alpm_db_get_pkgcache(handle->db_local); diff --git a/src/common/util-common.c b/src/common/util-common.c index 7145e49..c2422e0 100644 --- a/src/common/util-common.c +++ b/src/common/util-common.c @@ -73,6 +73,49 @@ char *mdirname(const char *path) return strdup("."); }

+/** Resolve the canonicalized absolute path of a symlink. + * @param path path to resolve + * @param resolved_path destination for the resolved path, will be malloc'd if + * NULL + * @return the resolved path + */ +char *lrealpath(const char *path, char *resolved_path) +{ + const char *bname = mbasename(path); + char *rpath = NULL, *dname = NULL; + int success = 0; + + if(strcmp(path, ".") == 0 || strcmp(path, "..") == 0) { + /* the entire path needs to be resolved */ + return realpath(path, resolved_path); + } + + if(!(dname = mdirname(path))) { + goto cleanup; + } + if(!(rpath = realpath(dname, NULL))) { + goto cleanup; + } + if(!resolved_path) { + if(!(resolved_path = malloc(strlen(rpath) + strlen(bname) + 2))) { + goto cleanup; + } + } + + strcpy(resolved_path, rpath); + if(resolved_path[strlen(resolved_path) - 1] != '/') { + strcat(resolved_path, "/"); + } + strcat(resolved_path, bname); + success = 1; + +cleanup: + free(dname); + free(rpath); + + return (success ? resolved_path : NULL); +} + #ifndef HAVE_STRNDUP /* A quick and dirty implementation derived from glibc */ /** Determines the length of a fixed-size string. diff --git a/src/common/util-common.h b/src/common/util-common.h index 04d4e9d..b3a4df1 100644 --- a/src/common/util-common.h +++ b/src/common/util-common.h @@ -22,6 +22,7 @@

const char *mbasename(const char *path); char *mdirname(const char *path); +char *lrealpath(const char *path, char *resolved_path);

#ifndef HAVE_STRNDUP char *strndup(const char *s, size_t n); diff --git a/test/pacman/tests/fileconflict007.py b/test/pacman/tests/fileconflict007.py index b61ddb4..7fe65ed 100644 --- a/test/pacman/tests/fileconflict007.py +++ b/test/pacman/tests/fileconflict007.py @@ -16,5 +16,3 @@ self.addrule("PKG_EXIST=pkg") self.addrule("PKG_VERSION=pkg|1.0-2") self.addrule("FILE_TYPE=dir/symdir/|dir") - -self.expectfailure = True diff --git a/test/pacman/tests/sync701.py b/test/pacman/tests/sync701.py index 912c794..590845f 100644 --- a/test/pacman/tests/sync701.py +++ b/test/pacman/tests/sync701.py @@ -19,5 +19,3 @@ self.addrule("PKG_VERSION=pkg1|1.0-2") self.addrule("PKG_EXIST=pkg2") self.addrule("FILE_TYPE=lib|dir") - -self.expectfailure = True diff --git a/test/pacman/tests/sync702.py b/test/pacman/tests/sync702.py index 8f4c0ad..c3e2320 100644 --- a/test/pacman/tests/sync702.py +++ b/test/pacman/tests/sync702.py @@ -19,5 +19,3 @@ self.addrule("PKG_VERSION=pkg2|1.0-2") self.addrule("PKG_EXIST=pkg1") self.addrule("FILE_TYPE=lib|dir") - -self.expectfailure = True

After the initial checks, we either use the path as a directory and have to append the trailing slash anyway or use it as a file in which case the trailing slash should be excluded. Signed-off-by: Andrew Gregory --- lib/libalpm/conflict.c | 12 ++++++------ test/pacman/tests/sync701.py | 2 -- test/pacman/tests/sync702.py | 2 -- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 3121cc7..d44a459 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -548,12 +548,12 @@ 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) */ - if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), filestr)) { + if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), relative_path)) { /* skip removal of file, but not add. this will prevent a second * package from removing the file when it was already installed * by its new owner (whether the file is in backup array or not */ handle->trans->skip_remove = - alpm_list_add(handle->trans->skip_remove, strdup(filestr)); + alpm_list_add(handle->trans->skip_remove, strdup(relative_path)); _alpm_log(handle, ALPM_LOG_DEBUG, "file changed packages, adding to remove skiplist\n"); resolved_conflict = 1; @@ -562,8 +562,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, /* check if all files of the dir belong to the installed pkg */ if(!resolved_conflict && S_ISDIR(lsbuf.st_mode) && dbpkg) { - char *dir = malloc(strlen(filestr) + 2); - sprintf(dir, "%s/", filestr); + char *dir = malloc(strlen(relative_path) + 2); + sprintf(dir, "%s/", relative_path); if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), dir)) { _alpm_log(handle, ALPM_LOG_DEBUG, "checking if all files in %s belong to %s\n", @@ -590,11 +590,11 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, } /* is the file unowned and in the backup list of the new package? */ - if(!resolved_conflict && _alpm_needbackup(filestr, p1)) { + if(!resolved_conflict && _alpm_needbackup(relative_path, p1)) { alpm_list_t *local_pkgs = _alpm_db_get_pkgcache(handle->db_local); int found = 0; for(k = local_pkgs; k && !found; k = k->next) { - if(alpm_filelist_contains(alpm_pkg_get_files(k->data), filestr)) { + if(alpm_filelist_contains(alpm_pkg_get_files(k->data), relative_path)) { found = 1; } } diff --git a/test/pacman/tests/sync701.py b/test/pacman/tests/sync701.py index 912c794..590845f 100644 --- a/test/pacman/tests/sync701.py +++ b/test/pacman/tests/sync701.py @@ -19,5 +19,3 @@ self.addrule("PKG_VERSION=pkg1|1.0-2") self.addrule("PKG_EXIST=pkg2") self.addrule("FILE_TYPE=lib|dir") - -self.expectfailure = True diff --git a/test/pacman/tests/sync702.py b/test/pacman/tests/sync702.py index 8f4c0ad..c3e2320 100644 --- a/test/pacman/tests/sync702.py +++ b/test/pacman/tests/sync702.py @@ -19,5 +19,3 @@ self.addrule("PKG_VERSION=pkg2|1.0-2") self.addrule("PKG_EXIST=pkg1") self.addrule("FILE_TYPE=lib|dir") - -self.expectfailure = True -- 1.8.2.2

Signed-off-by: Andrew Gregory --- Updated to include lrealpath definition and a buffer overflow check.. src/pacman/query.c | 137 +++++++++++++++++++++++++---------------------------- 1 file changed, 64 insertions(+), 73 deletions(-) diff --git a/src/pacman/query.c b/src/pacman/query.c index f051571..f5862a2 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -90,6 +90,49 @@ static void print_query_fileowner(const char *filename, alpm_pkg_t *info) } } +/** Resolve the canonicalized absolute path of a symlink. + * @param path path to resolve + * @param resolved_path destination for the resolved path, will be malloc'd if + * NULL + * @return the resolved path + */ +static char *lrealpath(const char *path, char *resolved_path) +{ + const char *bname = mbasename(path); + char *rpath = NULL, *dname = NULL; + int success = 0; + + if(strcmp(path, ".") == 0 || strcmp(path, "..") == 0) { + /* the entire path needs to be resolved */ + return realpath(path, resolved_path); + } + + if(!(dname = mdirname(path))) { + goto cleanup; + } + if(!(rpath = realpath(dname, NULL))) { + goto cleanup; + } + if(!resolved_path) { + if(!(resolved_path = malloc(strlen(rpath) + strlen(bname) + 2))) { + goto cleanup; + } + } + + strcpy(resolved_path, rpath); + if(resolved_path[strlen(resolved_path) - 1] != '/') { + strcat(resolved_path, "/"); + } + strcat(resolved_path, bname); + success = 1; + +cleanup: + free(dname); + free(rpath); + + return (success ? resolved_path : NULL); +} + static int query_fileowner(alpm_list_t *targets) { int ret = 0; @@ -133,11 +176,11 @@ static int query_fileowner(alpm_list_t *targets) packages = alpm_db_get_pkgcache(db_local); for(t = targets; t; t = alpm_list_next(t)) { - char *filename = NULL, *dname = NULL, *rpath = NULL; - const char *bname; + char *filename = NULL; + char rpath[PATH_MAX], *rel_path; struct stat buf; alpm_list_t *i; - size_t len, is_dir, bname_len, pbname_len; + size_t len, is_dir; unsigned int found = 0; if((filename = strdup(t->data)) == NULL) { @@ -165,82 +208,32 @@ static int query_fileowner(alpm_list_t *targets) } } - is_dir = S_ISDIR(buf.st_mode) ? 1 : 0; - if(is_dir) { - /* the entire filename is safe to resolve if we know it points to a dir, - * and it's necessary in case it ends in . or .. */ - dname = realpath(filename, NULL); - bname = mbasename(dname); - rpath = mdirname(dname); - } else { - bname = mbasename(filename); - dname = mdirname(filename); - rpath = realpath(dname, NULL); - } - - bname_len = strlen(bname); - pbname_len = bname_len + is_dir; - - if(!dname || !rpath) { + if(!lrealpath(filename, rpath)) { pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"), filename, strerror(errno)); goto targcleanup; } - for(i = packages; i && (!found || is_dir); i = alpm_list_next(i)) { - alpm_pkg_t *info = i->data; - alpm_filelist_t *filelist = alpm_pkg_get_files(info); - size_t j; - - for(j = 0; j < filelist->count; j++) { - const alpm_file_t *file = filelist->files + j; - char *ppath; - const char *pkgfile = file->name; - size_t pkgfile_len = strlen(pkgfile); - - /* make sure pkgfile and target are of the same type */ - if(is_dir != (pkgfile[pkgfile_len - 1] == '/')) { - continue; - } - - /* make sure pkgfile is long enough */ - if(pkgfile_len < pbname_len) { - continue; - } - - /* make sure pbname_len takes us to the start of a path component */ - if(pbname_len != pkgfile_len && pkgfile[pkgfile_len - pbname_len - 1] != '/') { - continue; - } + if(strncmp(rpath, path, rootlen) != 0) { + /* file is outside root, we know nothing can own it */ + pm_printf(ALPM_LOG_ERROR, _("No package owns %s\n"), filename); + goto targcleanup; + } - /* compare basename with bname */ - if(strncmp(pkgfile + pkgfile_len - pbname_len, bname, bname_len) != 0) { - continue; - } + rel_path = rpath + rootlen; - /* concatenate our dirname and the root path */ - if(rootlen + 1 + pkgfile_len - pbname_len > PATH_MAX) { - path[rootlen] = '\0'; /* reset path for error message */ - pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), path, pkgfile); - continue; - } - strncpy(path + rootlen, pkgfile, pkgfile_len - pbname_len); - path[rootlen + pkgfile_len - pbname_len] = '\0'; - - ppath = realpath(path, NULL); - if(!ppath) { - pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"), - path, strerror(errno)); - continue; - } + if((is_dir = S_ISDIR(buf.st_mode))) { + size_t rlen = strlen(rpath); + if(rlen + 2 >= PATH_MAX) { + pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath); + } + strcat(rpath + rlen, "/"); + } - if(strcmp(ppath, rpath) == 0) { - print_query_fileowner(filename, info); - found = 1; - free(ppath); - break; - } - free(ppath); + for(i = packages; i && (!found || is_dir); i = alpm_list_next(i)) { + if(alpm_filelist_contains(alpm_pkg_get_files(i->data), rel_path)) { + print_query_fileowner(rpath, i->data); + found = 1; } } if(!found) { @@ -252,8 +245,6 @@ targcleanup: ret++; } free(filename); - free(rpath); - free(dname); } return ret; -- 1.8.2.2