[PATCH i-g-t v3 4/5] lib/igt_core: Enhance __igt_fopen_data to support additional directories

[Kamil Konieczny]

Hi Naladala,
On 2025-02-19 at 03:23:48 +0530, Naladala Ramanaidu wrote:
> Add an additional parameter for the image directory. Refactor the
> function to use an array of directories to iterate through, reducing
> redundancy.
> 
> v2: Fix review comments.   (Santhosh, Kamil)
> v3: Update env variable name. (Kamil)
> 
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
> ---
>  lib/igt_core.c | 31 +++++++++++++++----------------
>  lib/igt_core.h |  6 +++---
>  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 407f7b551..20162ff55 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -3305,27 +3305,26 @@ void igt_reset_timeout(void)
>  	igt_set_timeout(0, NULL);
>  }
>  

You touch lib function so while you are at this please
add description.

> -FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
> -		       const char* filename)
> +FILE *__igt_fopen_data(const char *igt_srcdir, const char *igt_datadir,
> +		       const char *igt_imgdir, const char *filename)
>  {
>  	char path[PATH_MAX];
>  	FILE *fp;
> -
> -	snprintf(path, sizeof(path), "%s/%s", igt_datadir, filename);
> -	fp = fopen(path, "r");
> -	if (!fp) {
> -		snprintf(path, sizeof(path), "%s/%s", igt_srcdir, filename);
> -		fp = fopen(path, "r");
> -	}
> -	if (!fp) {
> -		snprintf(path, sizeof(path), "./%s", filename);
> -		fp = fopen(path, "r");
> +	size_t i;

Remove this var.

> +	const char *dirs[] = {igt_datadir, igt_srcdir, igt_imgdir,
> +			      getenv("IGT_DATA_PATH"), "./data"};
> +
> +	for (i = 0; i < ARRAY_SIZE(dirs); i++) {

Use: for (int i = 0; ...) {

> +		if (dirs[i]) {
> +			snprintf(path, sizeof(path), "%s/%s", dirs[i], filename);
> +			fp = fopen(path, "r");
> +			if (fp)
> +				return fp;

Use 'break;' instead, so no changes below around igt_critical

Rest looks good,

Regards,
Kamil

> +		}
>  	}
>  
> -	if (!fp)
> -		igt_critical("Could not open data file \"%s\": %m\n", filename);
> -
> -	return fp;
> +	igt_critical("Could not open data file \"%s\": %m\n", filename);
> +	return NULL;
>  }
>  
>  static void log_output(int *fd, enum igt_log_level level)
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index f0ba1a381..05134a104 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -1456,8 +1456,8 @@ static inline uint32_t igt_seconds_elapsed(struct timespec *start)
>  
>  void igt_reset_timeout(void);
>  
> -FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
> -		       const char* filename);
> +FILE *__igt_fopen_data(const char *igt_srcdir, const char *igt_datadir,
> +		       const char *igt_imgdir, const char *filename);
>  /**
>   * igt_fopen_data:
>   * @filename: filename to open.
> @@ -1466,7 +1466,7 @@ FILE *__igt_fopen_data(const char* igt_srcdir, const char* igt_datadir,
>   * then from build directory, and finally from current directory.
>   */
>  #define igt_fopen_data(filename) \
> -	__igt_fopen_data(IGT_SRCDIR, IGT_DATADIR, filename)
> +	__igt_fopen_data(IGT_SRCDIR, IGT_DATADIR, IGT_IMGDIR, filename)
>  
>  int igt_system(const char *command);
>  int igt_system_quiet(const char *command);
> -- 
> 2.43.0
>