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

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jan 08, 2021 at 06:00:42PM +0100, 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)) {

Hmmm... I think a helper class for this would make sense. I thus wonder
if we shouldn't revert 8f4e16f014c820a0.

Kieran, could you elaborate a little bit on the issue you've encountered
that led to that commit being developed ?

>  		char *ofPath = realpath(node.c_str(), nullptr);
>  		if (!ofPath)
>  			return {};

-- 
Regards,

Laurent Pinchart