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

[Laurent Pinchart]

Hi Kieran,

On Mon, Oct 24, 2022 at 04:30:05PM +0100, Kieran Bingham via libcamera-devel wrote:
> 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.

That's what I've just written in my separate reply to Nicholas' patch
:-)

There's another difference though, a non-existing file will log a Debug
message and a failure to open an Error message. As the HAL configuration
file is not mandatory, we should print an Error message if it's not
found.

> 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

-- 
Regards,

Laurent Pinchart