On Sun, Nov 20, 2011 at 12:18:59PM -0500, andrew.gregor...@gmail.com wrote:
> From: Andrew Gregory <andrew.gregor...@gmail.com>
>
> 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 <andrew.gregor...@gmail.com>
> ---
> 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.
> + if(filename[fnlen-1] != '/'){
> + filename = realloc(filename, sizeof(*filename)
> * (fnlen+2));
We use sizeof(type) rather than sizeof(*ptr).
> + 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?
> 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
pgpl1yJjZfpod.pgp
Description: PGP signature
