[igt-dev] [PATCH i-g-t] runner: Fix path handling

[Petri Latvala]

On Fri, Aug 10, 2018 at 04:44:46PM +0300, Arkadiusz Hiler wrote:
> absolute_path() tends to return NULL if more than the last element of
> the path is nonexistent. That behavior is confusing the callers, which
> use NULL as a convention for something not being set at all.
> 
> Let's fix that by sprinkling a little bit of recursion onto
> absolute_path() and wrapping POSIX in some sanity.
> 
> Cc: Petri Latvala <petri.latvala at intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>


Reviewed-by: Petri Latvala <petri.latvala at intel.com>


... dunno what I was thinking, having weird semantics and even unit
tests that make sure the semantics are weird.



> ---
>  runner/runner_tests.c | 22 ++++++--------------
>  runner/settings.c     | 47 ++++++++++++++++++++++++++-----------------
>  2 files changed, 34 insertions(+), 35 deletions(-)
> 
> diff --git a/runner/runner_tests.c b/runner/runner_tests.c
> index 7c662acc..89b4377a 100644
> --- a/runner/runner_tests.c
> +++ b/runner/runner_tests.c
> @@ -231,28 +231,18 @@ igt_main
>  		}
>  
>  		igt_subtest("absolute-path-converter") {
> -			struct {
> -				char *path;
> -				bool null;
> -			} data[] = { { "simple-name", false },
> -				     { "foo/bar", true },
> -				     { ".", false },
> -			};
> +			char paths[][15] = { "simple-name", "foo/bar", "." };
>  			size_t i;
>  
> -			for (i = 0; i < ARRAY_SIZE(data); i++) {
> +			for (i = 0; i < ARRAY_SIZE(paths); i++) {
>  				free(path);
> -				path = absolute_path(data[i].path);
> -				if (data[i].null) {
> -					igt_assert(path == NULL);
> -					continue;
> -				}
> +				path = absolute_path(paths[i]);
>  
>  				igt_assert(path[0] == '/');
> -				igt_debug("Got path %s for %s\n", path, data[i].path);
> +				igt_debug("Got path %s for %s\n", path, paths[i]);
>  				igt_assert(strstr(path, cwd) == path);
> -				if (strcmp(data[i].path, ".")) {
> -					igt_assert(strstr(path, data[i].path) != NULL);
> +				if (strcmp(paths[i], ".")) {
> +					igt_assert(strstr(path, paths[i]) != NULL);
>  				}
>  			}
>  		}
> diff --git a/runner/settings.c b/runner/settings.c
> index 31754a12..060459b0 100644
> --- a/runner/settings.c
> +++ b/runner/settings.c
> @@ -334,35 +334,44 @@ bool validate_settings(struct settings *settings)
>  	return true;
>  }
>  
> +static char *_dirname(const char *path)
> +{
> +	char *tmppath = strdup(path);
> +	char *tmpname = dirname(tmppath);
> +	tmpname = strdup(tmpname);
> +	free(tmppath);
> +	return tmpname;
> +}
> +
> +static char *_basename(const char *path)
> +{
> +	char *tmppath = strdup(path);
> +	char *tmpname = basename(tmppath);
> +	tmpname = strdup(tmpname);
> +	free(tmppath);
> +	return tmpname;
> +}
> +
>  char *absolute_path(char *path)
>  {
>  	char *result = NULL;
> -	char *tmppath, *tmpname;
> +	char *base, *dir;
> +	char *ret;
>  
>  	result = realpath(path, NULL);
>  	if (result != NULL)
>  		return result;
>  
> -	tmppath = strdup(path);
> -	tmpname = dirname(tmppath);
> -	free(result);
> -	result = realpath(tmpname, NULL);
> -	free(tmppath);
> +	dir = _dirname(path);
> +	ret = absolute_path(dir);
> +	free(dir);
>  
> -	if (result != NULL) {
> -		char *ret;
> +	base = _basename(path);
> +	asprintf(&result, "%s/%s", ret, base);
> +	free(base);
> +	free(ret);
>  
> -		tmppath = strdup(path);
> -		tmpname = basename(tmppath);
> -
> -		asprintf(&ret, "%s/%s", result, tmpname);
> -		free(result);
> -		free(tmppath);
> -		return ret;
> -	}
> -
> -	free(result);
> -	return NULL;
> +	return result;
>  }
>  
>  static char settings_filename[] = "metadata.txt";
> -- 
> 2.17.1
>