[pacman-dev] [PATCH 2/3] query_fileowner: resolve root early

[Allan McRae]

On 23/07/12 03:30, Andrew Gregory wrote:
> Resolving root early prevents later calls to realpath from having to do the
> work of actually resolving any symlinks in root.  Also removes the unnecessary
> root variable.
> 
> Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> ---
>  src/pacman/query.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index bb75465..84c9d3b 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -94,7 +94,6 @@ static int query_fileowner(alpm_list_t *targets)
>  {
>  	int ret = 0;
>  	char path[PATH_MAX];
> -	const char *root;
>  	size_t rootlen;
>  	alpm_list_t *t;
>  	alpm_db_t *db_local;
> @@ -108,14 +107,23 @@ static int query_fileowner(alpm_list_t *targets)
>  	/* Set up our root path buffer. We only need to copy the location of root in
>  	 * once, then we can just overwrite whatever file was there on the previous
>  	 * iteration. */
> -	root = alpm_option_get_root(config->handle);
> -	rootlen = strlen(root);
> -	if(rootlen + 1 > PATH_MAX) {
> -		/* we are in trouble here */
> -		pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, "");
> +
> +	/* resolve root now so any symlinks will only have to be resolved once */
> +	if(!realpath(alpm_option_get_root(config->handle), path)){
> +		pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"),
> +				alpm_option_get_root(config->handle), strerror(errno));

return 1;

> +	}
> +
> +	/* make sure there's enough room to append the package file to path */
> +	rootlen = strlen(path);
> +	if(rootlen + 2 > PATH_MAX) {
> +		pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), path, "");
>  		return 1;
>  	}
> -	strcpy(path, root);
> +
> +	/* append trailing '/' removed by realpath */
> +	path[rootlen++] = '/';
> +	path[rootlen] = '\0';
>  
>  	db_local = alpm_get_localdb(config->handle);
>  
> @@ -200,7 +208,8 @@ static int query_fileowner(alpm_list_t *targets)
>  				}
>  
>  				if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) {
> -					pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile);
> +					pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), path, pkgfile);
> +					continue;

That continue looks familiar.  Please build on top of you previous patch
to avoid confusion and make applying the patch easier:
https://patchwork.archlinux.org/patch/203/
(I know the confusion is mostly our fault given the number of patches
floating around in this area that have not been merged yet...)

>  				}
>  				/* concatenate our file and the root path */
>  				strcpy(path + rootlen, pkgfile);
> @@ -208,8 +217,15 @@ static int query_fileowner(alpm_list_t *targets)
>  				pdname = mdirname(path);
>  				ppath = realpath(pdname, NULL);
>  				free(pdname);
> +				path[rootlen] = '\0'; /* reset path for error messages */
> +
> +				if(!ppath) {
> +					pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"),
> +							ppath, strerror(errno));
> +					continue;
> +				}
>  
> -				if(ppath && strcmp(ppath, rpath) == 0) {
> +				if(strcmp(ppath, rpath) == 0) {
>  					print_query_fileowner(filename, info);
>  					found = 1;
>  				}
>