Re: [PATCH i-g-t] lib/kunit: Read results from debugfs

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Wed, Mar 27, 2024 at 10:54:53PM +0100, Janusz Krzysztofik wrote:
>> >+static DIR *kunit_debugfs_open(void)
>> >+{
>> >+	const char *debugfs_path = igt_debugfs_mount();
>> >+	int debugfs_fd, kunit_fd;
>> >+	DIR *kunit_dir;
>> >+
>> >+	if (igt_debug_on(!debugfs_path))
>> >+		return NULL;
>> >+
>> >+	debugfs_fd = open(debugfs_path, O_DIRECTORY);
>> >+	if (igt_debug_on(debugfs_fd < 0))
>> >+		return NULL;
>> >+
>> >+	kunit_fd = openat(debugfs_fd, "kunit", O_DIRECTORY);
>> >+	close(debugfs_fd);
>> >+	if (igt_debug_on(kunit_fd < 0))
>> >+		return NULL;
>> >+
>> >+	kunit_dir = fdopendir(kunit_fd);
>> >+	if (igt_debug_on(!kunit_dir))
>> >+		close(kunit_fd);
>> >+
>> >+	return kunit_dir;
>>
>>
>> any reason not to use strcat() + return fopen()
>
>To me the code looked simpler than if I copied and concatenated strings to a
>local buffer of fixed size with potential truncation handling.  I guess
>you prefer your pattern over mine, but you haven't explained why.  Would you
>like to convince me?

not really important. It just seems simpler to me, with no calls into
the kernel... completely untested, but something like:

	const char *debugfs_path = igt_debugfs_mount();
	char path[PATH_MAX];

	if (igt_debug_on(!debugfs_path || (strlen(debugfs_path) + strlen("/kunit") >= PATH_MAX)))
		return NULL;

	strcat(stpcpy(path, debugfs_path), "/kunit");

	return opendir(path);

Lucas De Marchi