[libcamera-devel] [PATCH 05/11] Removes references to std::filesystem, which Android 11's toolchain does not support.

[Laurent Pinchart]

Hi Nicholas,

On Mon, Oct 24, 2022 at 09:48:28AM -0500, Nicholas Roth via libcamera-devel wrote:
> I was kind of surprised to see this actually because right after
> creating the File object, we then check if it was successfully created
> and print an error otherwise. If I understand correctly, the
> std::filesystem-based check was redundant.
> 
> Please correct me if I’m wrong though.

Checking the file.open() return value will indeed catch this, but that
will result in a different behaviour. The current code reads as

	std::filesystem::path filePath = LIBCAMERA_SYSCONF_DIR;
	filePath /= "camera_hal.yaml";
	if (!std::filesystem::is_regular_file(filePath)) {
		LOG(HALConfig, Debug)
			<< "Configuration file: \"" << filePath << "\" not found";
		return -ENOENT;
	}

	File file(filePath);
	if (!file.open(File::OpenModeFlag::ReadOnly)) {
		int ret = file.error();
		LOG(HALConfig, Error) << "Failed to open configuration file "
				      << filePath << ": " << strerror(-ret);
		return ret;
	}

If the first check fails, the function prints a Debug message and
returns -ENOENT. The HAL configuration file is mandatory, so we don't
want to print an Error message in that case, it would confuse the user.
Then, if the file is found, but we can't open it, an error message is
printed, as that's not a valid situation.

This behaviour could be replicated by using file.exist() as Kieran
proposed, before opening the file object. There would still be a very
small difference in behaviour, as File::exist() will return true for
special files (pipes, character or block devices, ...) while
std::filesystem::is_regular_file() doesn't, but I think that's such a
corner case that it doesn't matter much.

By the way, if that's not too difficult in your e-mail client, could you
avoid removing all context when replying ? It makes reading your e-mails
more difficult otherwise.

-- 
Regards,

Laurent Pinchart