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

On Thu, Nov 6, 2008 at 10:17 PM, Allan McRae wrote:

...

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"))?

The problem was that subsequent calls to getenv are returning the same mem location. And we are mutilating that location (envpath) by calling strsep. So if we have more than one targets passed to Qo , then from second target onwards it will search only along the mutilated path.. I dont know if it is some mistake i made or the implimentation of getenv.

From man getenv As typically implemented, getenv() returns a pointer to a string within the environment list. The caller must take care not to modify

On Fri, Nov 7, 2008 at 1:18 PM, Jatheendra wrote: this string, since that would change the environment of the process.

About strdup(getenv("PATH")) , will it be possible to handle the error if getenv fails ? I am not sure what strdup(NULL) will do..

...

...

+ 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?

Will try :-)

...

...

}

+ 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; }

_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev