[pacman-dev] [PATCH] FS#8798 - Allow -Qo to perform a functional 'which'

[Allan McRae]

Jatheendra wrote:
> Hi all,
>
> This is a simple patch to add a "which" like functionality to pacman
> -Qo[Bug FS#8798]. This is my first patch ,so any constructive
> criticism is most welcome.
>
> Thanks,
> Shankar
>   

Nice work.  This is a really nice feature to implement.


> ---
>  src/pacman/query.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 0d48638..fb3bb92 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -55,6 +55,43 @@ static char *resolve_path(const char* file)
>  	return(str);
>  }
>
> +/* check if filename exists in PATH */
> +static int search_path(char *filename, struct stat * bufptr)
> +{
> +	char *envpath, *path, *fullname;
> +	int len;
> +
> +	if ((envpath = getenv("PATH")) == NULL) {
> +		return(-1);
> +	}
> +
> +	if ((envpath = strdup(envpath)) == NULL) {
> +		return(-1);
> +	}
> +	
>   

Do we need the strdup here?  I realize that getenv can return static 
data the may be overwritten by the next getenv, but we are not calling 
getenv again.  Or how about an all in one strdup(getenv("PATH"))?

> +	fullname = calloc(PATH_MAX+1, sizeof(char));
> +
> +	while ((path = strsep(&envpath, ":")) != NULL) {
> +			len = strlen(path);
> +
> +			/* strip trailing '/' */
> +			while (path[len-1] == '/') {
> +					path[--len] = '\0';
> +			}
> +			
> +			snprintf(fullname, PATH_MAX+1, "%s/%s", path, filename);
> +
> +			if(stat(fullname,bufptr) == 0) {
> +				strncpy(filename,fullname,PATH_MAX+1);
> +				free(fullname);
> +				return(0);
> +			}
>   

+1 to Aaron's comment here.

> +	}
> +	free(fullname);
> +	return(-1);
> +}
> +
> +
>  static int query_fileowner(alpm_list_t *targets)
>  {
>  	int ret = 0;
> @@ -66,21 +103,35 @@ static int query_fileowner(alpm_list_t *targets)
>  		return(1);
>  	}
>
> +	char *filename = calloc(PATH_MAX+1, sizeof(char));
> +
>  	for(t = targets; t; t = alpm_list_next(t)) {
>  		int found = 0;
> -		char *filename = alpm_list_getdata(t);
> +		strncpy(filename,alpm_list_getdata(t),PATH_MAX+1);
> +
>  		char *bname, *dname, *rpath;
>  		const char *root;
>  		struct stat buf;
>  		alpm_list_t *i, *j;
>
>  		if(stat(filename, &buf) == -1) {
> -			pm_fprintf(stderr, PM_LOG_ERROR, _("failed to read file '%s': %s\n"),
> -					filename, strerror(errno));
> -			ret++;
> -			continue;
> +			/*  if it is not a path but a program name, then check in PATH */
> +			if(strchr(filename,'/') == NULL) {
> +				if((search_path(filename,&buf)) == -1) {
> +					pm_fprintf(stderr, PM_LOG_ERROR, _("failed to read file '%s': %s\n"),
> +							filename, strerror(errno));
> +					ret++;
> +					continue;
> +				}
> +			} else {
> +				pm_fprintf(stderr, PM_LOG_ERROR, _("failed to read file '%s': %s\n"),
> +						filename, strerror(errno));
> +				ret++;
> +				continue;
> +			}
>   

Is there a way to have this error message not duplicated without making 
the if statement look really complicated?

>  		}
>
> +
>  		if(S_ISDIR(buf.st_mode)) {
>  			pm_fprintf(stderr, PM_LOG_ERROR,
>  				_("cannot determine ownership of a directory\n"));
> @@ -140,6 +191,7 @@ static int query_fileowner(alpm_list_t *targets)
>  		free(rpath);
>  	}
>
> +	free(filename);
>  	return ret;
>  }
>
>