On Fri, Nov 7, 2008 at 2:51 AM, Jatheendra <[email protected]> wrote: > On Fri, Nov 7, 2008 at 1:18 PM, Jatheendra <[email protected]> wrote: >> On Thu, Nov 6, 2008 at 10:17 PM, Allan McRae <[email protected]> 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 > 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; >>>> }
This looks like it got dropped on the floor (but is linked in the comments of FS#8798). Anyone want to dust it off and resubmit? -Dan
