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

[Allan McRae]

Nathan Wayde wrote:
> On 21/12/09 09:55, Allan McRae wrote:
>> When pacman queries the ownership of an object that is not a path,
>> it will check in the users PATH for a match. Implements FS#8798.
>>
>> Patch-by: Shankar<jatheendra at gmail.com>
>> [Allan: rework for master, tidy-up]
>> Signed-off-by: Allan McRae<allan at archlinux.org>
>> ---
>>
>> This was original posted a year ago...
>> http://mailman.archlinux.org/pipermail/pacman-dev/2008-November/007659.html 
>>
>>
>> The comments then were that we should maybe refactor out stuff dealing 
>> with
>> file paths, specifically removing trailing backslashes. That appears 
>> to only
>> occur in two other places and is very simple so I do not thing it needs
>> refactored and can be done in a spearate patch if necessary.
>>
>> The only other part I do not like is the "failed to read file" error 
>> block is
>> repeated, but I can not see a nice way to fix that.
>>
>>   src/pacman/query.c |   58 
>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 files changed, 53 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/pacman/query.c b/src/pacman/query.c
>> index 6b6a25d..6f579fd 100644
>> --- a/src/pacman/query.c
>> +++ b/src/pacman/query.c
>> @@ -56,6 +56,41 @@ 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);
>> +    }
>> +
>> +    fullname = calloc(PATH_MAX+1, sizeof(char));
>> +
>> +    while ((path = strsep(&envpath, ":")) != NULL) {
>> +        len = strlen(path);
>> +
>> +        /* strip the trailing slash if one exists */
>> +        if(path[len - 1] == '/') {
>> +                path[len - 1] = '\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);
>> +        }
>> +    }
>> +    free(fullname);
>> +    return(-1);
>> +}
>> +
> 
> just a heads-up, search_path() contains a memory leak; envpath is never 
> free()'d. Note, the original envpath is lost in the call to strsep().
> 

So it does... someone did not run valgrind on the patch :P

If I understand this correctly, I need to have another variable that 
keeps track of the pointer to the strdup object while the original gets 
split.  I.e.:

+/* check if filename exists in PATH */
+static int search_path(char *filename, struct stat * bufptr)
+{
+	char *envpath, *envpathsplit, *path, *fullname;
+	int len;
+
+	if ((envpath = getenv("PATH")) == NULL) {
+		return(-1);
+	}
+	if ((envpath = envpathsplit = strdup(envpath)) == NULL) {
+		return(-1);
+	}
+
+	fullname = calloc(PATH_MAX+1, sizeof(char));
+
+	while ((path = strsep(&envpathsplit, ":")) != NULL) {
+		len = strlen(path);
+
+		/* strip the trailing slash if one exists */
+		if(path[len - 1] == '/') {
+				path[len - 1] = '\0';
+		}
+
+		snprintf(fullname, PATH_MAX+1, "%s/%s", path, filename);
+
+		if(stat(fullname, bufptr) == 0) {
+			strncpy(filename, fullname, PATH_MAX+1);
+			free(envpath);
+			free(fullname);
+			return(0);
+		}
+	}
+	free(envpath);
+	free(fullname);
+	return(-1);
+}

That seems to make valgrind happy and keeps everything working.  Does 
that make sense or am I doing something stupid...

Updated patch on my working branch.

Allan