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

[Kieran Bingham]

Quoting Nicholas Roth (2022-10-24 15:48:28)
> 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.

It could be seen as redundant yes. I suspect it was to clearly mark the
difference between the file failing to open, and not being there. But as
"man 2 open" states:

        ENOENT O_CREAT is not set and the named file does not exist.

I expect that we should get a -ENOENT if the file isn't there when we
try to call File.open().

File.open() at [0] will only set O_CREAT if the mode is WriteOnly or
ReadWrite is used, and here we explicitly use ReadOnly at [1].

So I think removing the check entirely is fine and reasonable with a
clearly updated commit message, also explaining the redundancy and that
it's safe to remove without changing the underlying functionality.

Well it might be a very small subtle difference between the
implementation that's removed and returning -ENOENT if it doesn't exist,
as we're perhaps not checking that it's a "regular file" but I don't
think it's too crucial.

Perhaps a note in the commit that we no longer validate it's a regular
file, but expect it to be readable, and otherwise parsed by the
parseConfigFile() to validate the content.



[0] https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/base/file.cpp#n157
[1] https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_hal_config.cpp#n172

--
Kieran