On Sun, Nov 20, 2011 at 01:07:14PM -0500, Andrew Gregory wrote: > On Sun, 20 Nov 2011 12:39:11 -0500 > Dave Reisner <[email protected]> wrote: > > > On Sun, Nov 20, 2011 at 12:18:59PM -0500, [email protected] > > wrote: > > > From: Andrew Gregory <[email protected]> > > > > > > Not allowing fileowner queries for directories was an unnecessary > > > limitation. Queries for directories have poor performance due to > > > having to call real_path on every directory listed in every > > > package's file list. Because more than one package will likely own > > > a given directory, all packages are now checked instead of bailing > > > out after the first owner is found. > > > > > > Signed-off-by: Andrew Gregory <[email protected]> > > > --- > > > src/pacman/query.c | 17 +++++++++-------- > > > 1 files changed, 9 insertions(+), 8 deletions(-) > > > > > > diff --git a/src/pacman/query.c b/src/pacman/query.c > > > index 4c2ea81..8162662 100644 > > > --- a/src/pacman/query.c > > > +++ b/src/pacman/query.c > > > @@ -127,7 +127,7 @@ static int query_fileowner(alpm_list_t *targets) > > > * iteration. */ > > > root = alpm_option_get_root(config->handle); > > > rootlen = strlen(root); > > > - if(rootlen + 1 > PATH_MAX) { > > > + if(rootlen >= PATH_MAX) { > > > /* we are in trouble here */ > > > pm_printf(ALPM_LOG_ERROR, _("path too long: > > > %s%s\n"), root, ""); return 1; > > > @@ -164,12 +164,13 @@ static int query_fileowner(alpm_list_t > > > *targets) } > > > } > > > > > > + /* make sure directories end with '/' */ > > > if(S_ISDIR(buf.st_mode)) { > > > - pm_printf(ALPM_LOG_ERROR, > > > - _("cannot determine ownership of > > > directory '%s'\n"), filename); > > > - ret++; > > > - free(filename); > > > - continue; > > > + int fnlen = strlen(filename); > > > > strlen returns a size_t, not an int. Even though the compiler won't > > complain, we're pretty good about adhering to type correctness in the > > codebase. > > > > will fix > > > > + if(filename[fnlen-1] != '/'){ > > > + filename = realloc(filename, > > > sizeof(*filename) * (fnlen+2)); > > > > We use sizeof(type) rather than sizeof(*ptr). > > > > The HACKING file mentions, and I personally agree, that sizeof(*ptr) may > be better. Perhaps we should switch to sizeof(*ptr) or update the > HACKING file. > > > > + strcat(filename, "/"); > > > + } > > > } > > > > > > bname = mbasename(filename); > > > @@ -192,7 +193,7 @@ static int query_fileowner(alpm_list_t *targets) > > > } > > > free(dname); > > > > > > - for(i = alpm_db_get_pkgcache(db_local); i > > > && !found; i = alpm_list_next(i)) { > > > + for(i = alpm_db_get_pkgcache(db_local); i ; i = > > > alpm_list_next(i)) { > > > > This hurts performance of _all_ -Qo queries, which I'm not a fan of. > > Why not flag the query as needing to continue through the entire > > local DB specifically for directory queries? > > > > I considered that, but given that use of the -f flag could lead to more > than one package owning a file, I thought it would be best to check all > packages for files as well.
This is an DB inconsistency that we should not account for. Removal of a package (rightfully) doesn't check ownership by other packages, and will unconditionally remove every file in the filelist. We do not encourage usage of -f, and in fact the flag no longer exists -- only the longopt of '--force' remains. See 0d2600c5750. A tool to find multiple packages owning the same file would be^W^W is fairly straightforward to write: http://sprunge.us/XiGN. It might also be something to consider working into testdb since it already looks for databases which are broken in other ways... d > > > > alpm_pkg_t *info = i->data; > > > alpm_filelist_t *filelist = > > > alpm_pkg_get_files(info); size_t j; > > > @@ -214,7 +215,7 @@ static int query_fileowner(alpm_list_t *targets) > > > continue; > > > } > > > > > > - if(rootlen + 1 + strlen(pkgfile) > > > > PATH_MAX) { > > > + if(rootlen + strlen(pkgfile) >= > > > PATH_MAX) { pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), > > > root, pkgfile); } > > > /* concatenate our file and the > > > root path */ -- > > > 1.7.7.3 > > > > > > > dave > >
pgpzwpcY60zHv.pgp
Description: PGP signature
