[lttng-dev] [lttng-tools PATCH 3/4] Correct the behavior of the utils_expand_path function

[mathieu.desnoyers@efficios.com (Mathieu Desnoyers)]

----- Original Message -----
> From: "Rapha?l Beamonte" <raphael.beamonte@gmail.com>
> To: dgoulet at efficios.com
> Cc: "Rapha?l Beamonte" <raphael.beamonte at gmail.com>, lttng-dev at lists.lttng.org
> Sent: Wednesday, November 13, 2013 12:34:37 AM
> Subject: [lttng-dev] [lttng-tools PATCH 3/4] Correct the behavior of the	utils_expand_path function
> 
> Even if the utils_expand_path function was intended to allow to
> use unexistent directory paths, it was in fact only working for
> some kind of arguments. Paths like "foo", "bar/" or "bar/foo"
> when the "bar" directory does not exist wasn't working. This
> patch introduce a new way to expand paths in this function that
> also works properly for these kind of arguments.

I might be missing something, but how about, instead:

1) if path start with /, directly pass it to realpath()
2) if path does not start with /, prepend getcwd() to it, and then call realpath() on the result.

If we can do the same result as this patch without reimplementing tricky string handling, we should really aim for re-using existing libraries.

Thoughts ?

Thanks,

Mathieu

> 
> Signed-off-by: Rapha?l Beamonte <raphael.beamonte at gmail.com>
> ---
>  src/common/utils.c |   86
>  ++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 64 insertions(+), 22 deletions(-)
> 
> diff --git a/src/common/utils.c b/src/common/utils.c
> index 4ccf36a..6938a5a 100644
> --- a/src/common/utils.c
> +++ b/src/common/utils.c
> @@ -112,7 +112,7 @@ error:
>  LTTNG_HIDDEN
>  char *utils_expand_path(const char *path)
>  {
> -	const char *end_path = path;
> +	const char *end_path = NULL;
>  	char *next, *cut_path = NULL, *expanded_path = NULL;
>  
>  	/* Safety net */
> @@ -120,38 +120,80 @@ char *utils_expand_path(const char *path)
>  		goto error;
>  	}
>  
> -	/* Find last token delimited by '/' */
> -	while ((next = strpbrk(end_path + 1, "/"))) {
> -		end_path = next;
> -	}
> -
> -	/* Cut last token from original path */
> -	cut_path = strndup(path, end_path - path);
> -
> +	/* Allocate memory for the expanded path */
>  	expanded_path = zmalloc(PATH_MAX);
>  	if (expanded_path == NULL) {
>  		PERROR("zmalloc expand path");
>  		goto error;
>  	}
>  
> -	expanded_path = realpath((char *)cut_path, expanded_path);
> -	if (expanded_path == NULL) {
> -		switch (errno) {
> -		case ENOENT:
> -			ERR("%s: No such file or directory", cut_path);
> -			break;
> -		default:
> -			PERROR("realpath utils expand path");
> -			break;
> +	/* If given path is already absolute */
> +	if (*path == '/') {
> +		strncpy(expanded_path, path, PATH_MAX);
> +	/* Else, we have some work to do */
> +	} else {
> +		/* Pointer to the last char of the path */
> +		const char *last_char = path + strlen(path) - 1;
> +
> +		end_path = path;
> +
> +		/* Split part that will be resolved by realpath (relative path from
> +		 * current directory using ./ or ../ only) and part that could not
> +		 * (directory names)
> +		 */
> +		while ((next = strpbrk(end_path, "/")) && (next != last_char)) {
> +			end_path = next + 1;
> +			if (strncmp(end_path, "./", 2) != 0 &&
> +					strncmp(end_path, "../", 3) != 0) {
> +				break;
> +			}
> +		}
> +
> +		/* If this is the end of the string, and we still can resolve it */
> +		if (strncmp(end_path, "..\0", 3) == 0 ||
> +				strncmp(end_path, ".\0", 2) == 0) {
> +			end_path += strlen(end_path);
> +		}
> +
> +		/* If the end part is the whole path, we are in the current dir */
> +		if (end_path == path) {
> +			cut_path = strdup(".");
> +		/* Else, cut the resolvable part from original path */
> +		} else {
> +			cut_path = strndup(path, end_path - path);
> +		}
> +
> +		/* Resolve the canonical path of the first part of the path */
> +		expanded_path = realpath((char *)cut_path, expanded_path);
> +		if (expanded_path == NULL) {
> +			switch (errno) {
> +			case ENOENT:
> +				ERR("%s: No such file or directory", cut_path);
> +				break;
> +			default:
> +				PERROR("realpath utils expand path");
> +				break;
> +			}
> +			goto error;
> +		}
> +
> +		/* Add end part to expanded path if not empty */
> +		if (*end_path != 0) {
> +			strncat(expanded_path, "/", PATH_MAX - strlen(expanded_path) - 1);
> +			strncat(expanded_path, end_path,
> +					PATH_MAX - strlen(expanded_path) - 1);
>  		}
> -		goto error;
>  	}
>  
> -	/* Add end part to expanded path */
> -	strncat(expanded_path, end_path, PATH_MAX - strlen(expanded_path) - 1);
> +	/* Resolve the internal './' and '../' strings */
> +	next = utils_resolve_relative(expanded_path);
> +	if (next == NULL) {
> +		goto error;
> +	}
>  
> +	free(expanded_path);
>  	free(cut_path);
> -	return expanded_path;
> +	return next;
>  
>  error:
>  	free(expanded_path);
> --
> 1.7.10.4
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com