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

Jatheendra jatheendra at gmail.com
Fri Nov 7 02:51:02 EST 2008


On Fri, Nov 7, 2008 at 1:18 PM, Jatheendra <jatheendra at gmail.com> wrote:
> On Thu, Nov 6, 2008 at 10:17 PM, Allan McRae <allan at archlinux.org> 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;
>>>  }
>>>
>>>
>>
>>
>>
>> _______________________________________________
>> pacman-dev mailing list
>> pacman-dev at archlinux.org
>> http://archlinux.org/mailman/listinfo/pacman-dev
>>
>



More information about the pacman-dev mailing list