[libcamera-devel] [PATCH] libcamera: sysfs: Fix directory exists check

[Niklas Söderlund]

Hi Sebastian,

Thanks for testing patch.

On 2021-01-09 07:12:23 +0100, Sebastian Fricke wrote:
> Hey Niklas,
> 
> Thank you for the patch.
> I have tested your patch on my FriendlyElec NanoPC-T4 development board
> with the OV13850.
> *Before* I applied your patch the output looked like this:
> ```
> basti at nanopct4:~$ cam -l
> Available cameras:
> 1: Internal front camera (platform/ff110000.i2c/i2c-1/1-0010 ov13850)
> ```
> And *after* applying your patch it looks like this:
> ```
> basti at nanopct4:~$ cam -l
> Available cameras:
> 1: Internal front camera (/base/i2c at ff110000/ov13850 at 10)
> ```

It looks like the patch solves the persistent naming regression on your 
platform as well. If you test a camera enumeration before  
8f4e16f014c820a0 dose it match the later case 
(/base/i2c at ff110000/ov13850 at 10) ?

> 
> I also tested this patch on my laptop:
> *Before*:
> ```
> basti at basti-TUXEDO-Book-XA1510:~$ cam -l
> Available cameras:
> 1: External camera 'Chicony USB2.0 Camera: Chicony ' (\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)
> ```
> *After*:
> ```
> basti at basti-TUXEDO-Book-XA1510:~$ cam -l
> Available cameras:
> 1: External camera 'Chicony USB2.0 Camera: Chicony ' (\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)
> ```
> 
> The output on my laptop looks a little odd, but that is another issue.

What is odd about your laptop output?

> 
> Greetings
> 
> Sebastian
> 
> On 08.01.2021 18:00, Niklas Söderlund wrote:
> > The scope of File::exists() was changed to only validate that a file
> > exists and is therefore not usable to check if a directory exists. This
> > breaks the persistent name generation for DT based systems as it uses
> > File::exists() to check for directories, fix this by using stat()
> > directly.
> > 
> > On Scarlet the persistent names of the cameras are impacted by this and
> > where incorrectly reported as:
> > 
> >    $ cam -l
> >    Available cameras:
> >    1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)
> >    2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695
> > 
> > While the expected ones are restored with this fix:
> > 
> >    $ cam -l
> >    Available cameras:
> >    1: Internal front camera (/base/i2c at ff160000/camera at 3c)
> >    2: Internal front camera (/base/i2c at ff160000/camera at 36)
> > 
> > Fixes: 8f4e16f014c820a0 ("test: file: Check that directories are not treated as files")
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > src/libcamera/sysfs.cpp | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp
> > index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644
> > --- a/src/libcamera/sysfs.cpp
> > +++ b/src/libcamera/sysfs.cpp
> > @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)
> > std::string firmwareNodePath(const std::string &device)
> > {
> > 	std::string fwPath, node;
> > +	struct stat st;
> > 
> > 	/* Lookup for DT-based systems */
> > 	node = device + "/of_node";
> > -	if (File::exists(node)) {
> > +	if (!stat(node.c_str(), &st)) {
> > 		char *ofPath = realpath(node.c_str(), nullptr);
> > 		if (!ofPath)
> > 			return {};
> > -- 
> > 2.30.0
> > 
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund