[pacman-dev] [PATCH] allow querying fileowner for directories

[Dave Reisner]

On Sun, Nov 20, 2011 at 12:18:59PM -0500, andrew.gregory.8 at gmail.com wrote:
> From: Andrew Gregory <andrew.gregory.8 at 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.gregory.8 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://mailman.archlinux.org/pipermail/pacman-dev/attachments/20111120/2caeeb9a/attachment.asc>