[PATCH i-g-t] runner: Parse results harder

Krzysztof Karas krzysztof.karas at intel.com
Mon Mar 3 11:02:50 UTC 2025


Hi Kamil,

[...]

> +bool open_output_files_rdonly(int dirfd, int *fds)
> +{
> +	bool ret = true;
> +
> +	for (int i = 0; i < _F_LAST; i++)
> +		if ((fds[i] = openat(dirfd, filenames[i], O_RDONLY)) < 0) {

You could use `open_for_reading()` function, which already calls
openat with O_RDONLY flag.

[...]

> +		if (fsizes[_F_SOCKET] || fsizes[_F_OUT] || fsizes[_F_ERR] ||
> +		    fsizes[_F_DMESG])
> +			valid_out_files = true;
> +		else
> +			valid_out_files = false;
> +
> +		if (!valid_out_files) {
> +			/* no output saved in any file */
> +			fprintf(stderr, "results: no valid output files\n");
> +			close_outputs(fds);
> +
> +			return false;
> +		}

`valid_out_files` flag is immediately used after setting and not
seen later in the code. The irst `if` could check if all
fsizes[...] are empty and then the code could look like:

if (!fsizes[_F_SOCKET] && !fsizes[_F_OUT] && !fsizes[_F_ERR] &&
	!fsizes[_F_DMESG]) {
	/* no output saved in any file */
	fprintf(stderr, "results: no valid output files\n");
	close_outputs(fds);

	return false;
}

Apart from these two nits the code looks good. I like that you
removed early returning, so now we'll be getting more logs,
even if something breaks earlier.

Best Regards,
Krzysztof



More information about the igt-dev mailing list