[pacman-dev] [PATCH 2/8] alpm_filelist: remove resolved_path
Andrew Gregory
andrew.gregory.8 at gmail.com
Fri Apr 26 20:00:23 EDT 2013
Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
---
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
More information about the pacman-dev
mailing list