[PATCH i-g-t v3 3/5] runner/settings: Add function to set IGT_DATA_PATH environment variable

[Kamil Konieczny]

Hi Naladala,
On 2025-02-19 at 03:23:47 +0530, Naladala Ramanaidu wrote:
> Add environment variable to obtain absolute path for PNG files
> at runtime.
> 
> v2: Fix review comments. (Kamil)
> v3: Fix review comments. (kamil)
> 
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
> ---
>  runner/settings.c | 13 +++++++++++++
>  runner/settings.h |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/runner/settings.c b/runner/settings.c
> index c1f3be77a..77fabafb0 100644
> --- a/runner/settings.c
> +++ b/runner/settings.c
> @@ -601,6 +601,15 @@ char *absolute_path(const char *path)
>  	return result;
>  }
>  
> +int set_runner_datadir(void)

imho better: set_tests_datadir()

> +{
> +	const char *datapath = "../share/igt-gpu-tools";

On my test container I have binaries in

Installing tests/kms_3d to /usr/local/libexec/igt-gpu-tools

while data (testlists, PNG and blocklists) in

/usr/local/share/igt-gpu-tools/

so this would be 
	const char *datapath = "../../share/igt-gpu-tools";

Maybe just check for these two? Also consider developer
running from tests/ folder with data in ../data or ../../data

> +	char *abpath;
> +

Check if IGT_DATA_PATH is already set with getenv()
so no action is needed in such case.

> +	abpath = absolute_path(datapath);

if abpath is NULL do nothing or check alternative.

> +	return setenv("IGT_DATA_PATH", abpath, 1);
> +}
> +
>  static char *bin_path(char *fname)
>  {
>  	char *path, *p;
> @@ -643,6 +652,8 @@ void init_settings(struct settings *settings)
>  {
>  	memset(settings, 0, sizeof(*settings));
>  	IGT_INIT_LIST_HEAD(&settings->env_vars);
> +	if (set_runner_datadir())

setenv() returns -1 on error

> +		fprintf(stderr, "Data dir path not set\n");
>  	igt_vec_init(&settings->hook_strs, sizeof(char *));
>  }
>  
> @@ -660,6 +671,8 @@ void clear_settings(struct settings *settings)
>  	free_hook_strs(&settings->hook_strs);
>  	free_array_deep((void **)settings->cmdline.argv, settings->cmdline.argc);
>  
> +	unsetenv("IGT_DATA_PATH");

Use this only when you succesfully used setenv(), I suggest
using settings structure for this.

Regards,
Kamil

> +
>  	init_settings(settings);
>  }
>  
> diff --git a/runner/settings.h b/runner/settings.h
> index 42c96665a..7b8292509 100644
> --- a/runner/settings.h
> +++ b/runner/settings.h
> @@ -154,5 +154,6 @@ bool serialize_settings(struct settings *settings);
>  
>  bool read_settings_from_file(struct settings *settings, FILE* f);
>  bool read_settings_from_dir(struct settings *settings, int dirfd);
> +int set_runner_datadir(void);
>  
>  #endif
> -- 
> 2.43.0
>