[lttng-dev] [lttng-tools PATCH 1/4] Introduce a new utils_resolve_relative function

[Mathieu Desnoyers]

Some comments...

----- Original Message -----
> From: "Raphaël Beamonte" <raphael.beamonte at 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:35 AM
> Subject: [lttng-dev] [lttng-tools PATCH 1/4] Introduce a new	utils_resolve_relative function
> 
> This function aims to resolve relative path such as './' and
> '../' in the middle of a path string. This allows to use paths
> such as '~/../test' that are received as '/home/x/../test' for
> instance.
> 
> Signed-off-by: Raphaël Beamonte <raphael.beamonte at gmail.com>
> ---
>  src/common/utils.c |   68
>  ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/common/utils.h |    1 +
>  2 files changed, 69 insertions(+)
> 
> diff --git a/src/common/utils.c b/src/common/utils.c
> index da4c036..4ccf36a 100644
> --- a/src/common/utils.c
> +++ b/src/common/utils.c
> @@ -36,6 +36,74 @@
>  #include "defaults.h"
>  
>  /*
> + * Resolve the './' and '../' strings in the middle of a path using
> + * our very own way to do it, so that it works even if the directory
> + * does not exist

We should document that this returns allocated memory which needs to be freed by the caller.

Also, we should clearly state that the only reason why we implement this is that realpath() only works on existing paths, which does not cover our use-case of having to resolve partially unexisting paths.

> + */
> +LTTNG_HIDDEN
> +char *utils_resolve_relative(const char *path)
> +{
> +	char *next, *previous, *slash, *start_path, *absolute_path = NULL;
> +
> +	/* Safety net */
> +	if (path == NULL) {
> +		goto error;
> +	}
> +
> +	/* Allocate memory for the absolute path */
> +	absolute_path = zmalloc(PATH_MAX);
> +	if (absolute_path == NULL) {
> +		PERROR("zmalloc expand path");
> +		goto error;
> +	}
> +
> +	/* Copy the path in the absolute path */
> +	strncpy(absolute_path, path, PATH_MAX);
> +
> +	/* As long as we find '/./' in the path string */
> +	while ((next = strstr(absolute_path, "/./"))) {
> +
> +		/* We prepare the start_path not containing it */
> +		start_path = strndup(absolute_path, next - absolute_path);
> +
> +		/* And we concatenate it with the part after this string */
> +		snprintf(absolute_path, PATH_MAX, "%s%s", start_path, next + 2);
> +
> +		free(start_path);
> +	}
> +
> +	/* As long as we find '/../' in the path string */
> +	while ((next = strstr(absolute_path, "/../"))) {
> +		/* If the path starts with '/../', there's a problem */
> +		if (next == absolute_path) {
> +			ERR("%s: Path cannot be resolved", path);
> +			goto error;
> +		}
> +
> +		/* We find the last level of directory */
> +		previous = absolute_path;
> +		while ((slash = strpbrk(previous + 1, "/")) && slash != next) {
> +			previous = slash;
> +		}
> +
> +		/* Then we prepare the start_path not containing it */
> +		start_path = strndup(absolute_path, previous - absolute_path);
> +
> +		/* And we concatenate it with the part after the '/../' */
> +		snprintf(absolute_path, PATH_MAX, "%s%s", start_path, next + 3);
> +
> +		free(start_path);
> +	}

I might be missing something, but how can this work on paths like this ?

"./a/../b/./c/../d/./e" ?

At first glance, it looks like the only cases that work correctly are those in the test case, but there are various variants that won't.

One possible alternative approach to all this path mangling would be to use realpath() on sections of paths (using / as separator, from the start of path), until realpath() fails to lookup the final paths. Maybe then could we simply append the paths as is without resolving. Users would have to know that they should not put "." or ".." in the unexisting part of their paths. Thoughts ?

Thanks,

Mathieu

> +
> +	return absolute_path;
> +
> +error:
> +	free(absolute_path);
> +	return NULL;
> +}
> +
> +
> +/*
>   * Return the realpath(3) of the path even if the last directory token does
>   not
>   * exist. For example, with /tmp/test1/test2, if test2/ does not exist but
>   the
>   * /tmp/test1 does, the real path is returned. In normal time, realpath(3)
> diff --git a/src/common/utils.h b/src/common/utils.h
> index 52f2798..c56942f 100644
> --- a/src/common/utils.h
> +++ b/src/common/utils.h
> @@ -26,6 +26,7 @@
>  #define MEBI_LOG2 20
>  #define GIBI_LOG2 30
>  
> +char *utils_resolve_relative(const char *path);
>  char *utils_expand_path(const char *path);
>  int utils_create_pipe(int *dst);
>  int utils_create_pipe_cloexec(int *dst);
> --
> 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