[{"id":14484,"web_url":"https://patchwork.libcamera.org/comment/14484/","msgid":"","date":"2021-01-09T01:38:08","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, Jan 08, 2021 at 06:00:42PM +0100, Niklas Söderlund wrote:\n> The scope of File::exists() was changed to only validate that a file\n> exists and is therefore not usable to check if a directory exists. This\n> breaks the persistent name generation for DT based systems as it uses\n> File::exists() to check for directories, fix this by using stat()\n> directly.\n> \n> On Scarlet the persistent names of the cameras are impacted by this and\n> where incorrectly reported as:\n> \n> $ cam -l\n> Available cameras:\n> 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n> 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n> \n> While the expected ones are restored with this fix:\n> \n> $ cam -l\n> Available cameras:\n> 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n> 2: Internal front camera (/base/i2c@ff160000/camera@36)\n> \n> Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/sysfs.cpp | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n> --- a/src/libcamera/sysfs.cpp\n> +++ b/src/libcamera/sysfs.cpp\n> @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n> std::string firmwareNodePath(const std::string &device)\n> {\n> \tstd::string fwPath, node;\n> +\tstruct stat st;\n> \n> \t/* Lookup for DT-based systems */\n> \tnode = device + \"/of_node\";\n> -\tif (File::exists(node)) {\n> +\tif (!stat(node.c_str(), &st)) {\n\nHmmm... I think a helper class for this would make sense. I thus wonder\nif we shouldn't revert 8f4e16f014c820a0.\n\nKieran, could you elaborate a little bit on the issue you've encountered\nthat led to that commit being developed ?\n\n> \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n> \t\tif (!ofPath)\n> \t\t\treturn {};","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 09A7BC0F1A\n\tfor ;\n\tSat, 9 Jan 2021 01:38:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C2AC68060;\n\tSat, 9 Jan 2021 02:38:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0644A6010A\n\tfor ;\n\tSat, 9 Jan 2021 02:38:22 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 797EDDA;\n\tSat, 9 Jan 2021 02:38:21 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"X1m1Z/BH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610156301;\n\tbh=9/WKvUU5JjDtpnrVIgparO87bw1mf4lRPw7h4/ufFZ0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X1m1Z/BHA/L+ujBSe5eqOR+VS7K+LH12vSIwc+ZUEXxZuQgwTcrX5bG4yMwy3b1eR\n\tUO2Hy4gAfjr53TemG4rWpdA6S4JKID/Z28nF++remc7NUFf9PDK9fMKqKDYJrByCBG\n\tBkyAT8RG3hHG7otFXbPzhqrCOwfqSTqvvtm7qyHY=","Date":"Sat, 9 Jan 2021 03:38:08 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14485,"web_url":"https://patchwork.libcamera.org/comment/14485/","msgid":"","date":"2021-01-09T02:33:43","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2021-01-09 03:38:08 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Fri, Jan 08, 2021 at 06:00:42PM +0100, Niklas Söderlund wrote:\n> > The scope of File::exists() was changed to only validate that a file\n> > exists and is therefore not usable to check if a directory exists. This\n> > breaks the persistent name generation for DT based systems as it uses\n> > File::exists() to check for directories, fix this by using stat()\n> > directly.\n> > \n> > On Scarlet the persistent names of the cameras are impacted by this and\n> > where incorrectly reported as:\n> > \n> > $ cam -l\n> > Available cameras:\n> > 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n> > 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n> > \n> > While the expected ones are restored with this fix:\n> > \n> > $ cam -l\n> > Available cameras:\n> > 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n> > 2: Internal front camera (/base/i2c@ff160000/camera@36)\n> > \n> > Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/sysfs.cpp | 3 ++-\n> > 1 file changed, 2 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> > index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n> > --- a/src/libcamera/sysfs.cpp\n> > +++ b/src/libcamera/sysfs.cpp\n> > @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n> > std::string firmwareNodePath(const std::string &device)\n> > {\n> > \tstd::string fwPath, node;\n> > +\tstruct stat st;\n> > \n> > \t/* Lookup for DT-based systems */\n> > \tnode = device + \"/of_node\";\n> > -\tif (File::exists(node)) {\n> > +\tif (!stat(node.c_str(), &st)) {\n> \n> Hmmm... I think a helper class for this would make sense. I thus wonder\n> if we shouldn't revert 8f4e16f014c820a0.\n\nI'm starting to lean towards that some helpers are not particular \nhelpful :-) I primarily think our File class adds value as it eases \nusage of map() and unmap(). Wrapping seek(), read() and write() to \nbetter cope with C++ data structures is nice but could perhaps be done \nas streams if it was just that? I think 8f4e16f014c820a0 adds value as \nit restricts the File class to only work with files.\n\nWe could of add a Directory::exists() helper to deal with this. But I \nthink that is going down a bad road as I'm sure someone at some point \nwant to just check if a path exists and don't really care if it's a \ndirectory or a file (or symlink). Should we create a helper for each \ntype of lookup or have implementations use multiple helpers at the cost \nof performance? \n\nI'm think it would be cleaner to use primitives of our environment to \ndeal with things that require no context or cleanup outside of the local \nscope. It's easier to 'man stat' while reading the code then grepping \naround in the code base for exactly what File::exists() checks for.\n\nThat being said I do not feel strongly about this particular patch it \njust happened to align with my over all feeling for similar situations.\n\n> \n> Kieran, could you elaborate a little bit on the issue you've encountered\n> that led to that commit being developed ?\n> \n> > \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n> > \t\tif (!ofPath)\n> > \t\t\treturn {};\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 50D5DC3256\n\tfor ;\n\tSat, 9 Jan 2021 02:33:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D230768062;\n\tSat, 9 Jan 2021 03:33:46 +0100 (CET)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 193206010A\n\tfor ;\n\tSat, 9 Jan 2021 03:33:46 +0100 (CET)","by mail-lf1-x135.google.com with SMTP id u25so6807750lfc.2\n\tfor ;\n\tFri, 08 Jan 2021 18:33:46 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tw25sm676475lfk.237.2021.01.08.18.33.43\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 08 Jan 2021 18:33:44 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"XE7lsA7h\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=/YxQOa/MMlB42ACfEsx5h+FlTBlxzROajuHrsx31isk=;\n\tb=XE7lsA7hU0gQFUkKvpwWKHrgoxZF+xwUcOmLCONORooYpTdEKH39BaeDPGYjzqrIgm\n\t0Youn376XLB33dw07nK4tG+T7s4mFu0/981nIkAkEkdGpHC/OJv9h/93iZNUSzuiKuoQ\n\tWgBKdW0y0nHxZLVSWC8SVo93WkXTJLwkrCxUQl+v8wh2y3+XaE77e6pAMd7dCrmgMPmM\n\tmnsbNQbuzFXwMKswMwVjEDEdYli4x09DEb+flbfXkDyJHCGXC/a7rFDBMD77wLEBqr2l\n\tZ2gmP+qfYO4o7z5LlgqXN6/3+Fc2EFBxBr847V/s6m/rJhOH+QpcjAlunBNb2Wrigjdm\n\tGvAg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=/YxQOa/MMlB42ACfEsx5h+FlTBlxzROajuHrsx31isk=;\n\tb=UHfouFBlMRONptzLlNAMQo6tzfTjYFIBxHWiQE8BMeXPfhsQbT71pIB7QebK39XsG3\n\tPuSEfSpFN+l2//mrXUkq2tNhg40kijTRKp98NnKvlhpx3uvxyQ/2IOW8TXp4a7brX1q1\n\tMhxrB3OpEB9bXXxGe6+usOsqy9/ueNqAL0pxZgQntGZprMpOp6DOsF1RdkV+rYMQjznD\n\t9k9rOYzbNDob+2lf8t5O5fC5wchtc34+DFFdnMY5beH8BUN8aDP4qrK342UZ06dyB4Jk\n\tCQIFBSCPqNx0oCefVOJG6aYFkbP36gpYvL/ZOUl/Rmv1hHj0wvCe4z5xMvf6ScZvsk1x\n\tw8YA==","X-Gm-Message-State":"AOAM530XtCRtNJ2pmzOwOoW5/hlsZy0PCrE0V9gWV0AiVsTgd4WyR4uC\n\tvj70Y843r/JNz6tL/ZJ6+N06jw==","X-Google-Smtp-Source":"ABdhPJxaYPtr0caZHk4AsyksKtJk/crXLKZrW8Y3PvA0zlXji5pFS52z2wDoHxej90vlY8kH2hBF8A==","X-Received":"by 2002:a19:8b8b:: with SMTP id\n\tn133mr2926819lfd.278.1610159625192; \n\tFri, 08 Jan 2021 18:33:45 -0800 (PST)","Date":"Sat, 9 Jan 2021 03:33:43 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14486,"web_url":"https://patchwork.libcamera.org/comment/14486/","msgid":"<20210109061223.mxq7obfiwad44cwm@basti-TUXEDO-Book-XA1510>","date":"2021-01-09T06:12:23","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":73,"url":"https://patchwork.libcamera.org/api/people/73/","name":"Sebastian Fricke","email":"sebastian.fricke.linux@gmail.com"},"content":"Hey Niklas,\n\nThank you for the patch.\nI have tested your patch on my FriendlyElec NanoPC-T4 development board\nwith the OV13850.\n*Before* I applied your patch the output looked like this:\n```\nbasti@nanopct4:~$ cam -l\nAvailable cameras:\n1: Internal front camera (platform/ff110000.i2c/i2c-1/1-0010 ov13850)\n```\nAnd *after* applying your patch it looks like this:\n```\nbasti@nanopct4:~$ cam -l\nAvailable cameras:\n1: Internal front camera (/base/i2c@ff110000/ov13850@10)\n```\n\nI also tested this patch on my laptop:\n*Before*:\n```\nbasti@basti-TUXEDO-Book-XA1510:~$ cam -l\nAvailable cameras:\n1: External camera 'Chicony USB2.0 Camera: Chicony ' (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n```\n*After*:\n```\nbasti@basti-TUXEDO-Book-XA1510:~$ cam -l\nAvailable cameras:\n1: External camera 'Chicony USB2.0 Camera: Chicony ' (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n```\n\nThe output on my laptop looks a little odd, but that is another issue.\n\nGreetings\n\nSebastian\n\nOn 08.01.2021 18:00, Niklas Söderlund wrote:\n>The scope of File::exists() was changed to only validate that a file\n>exists and is therefore not usable to check if a directory exists. This\n>breaks the persistent name generation for DT based systems as it uses\n>File::exists() to check for directories, fix this by using stat()\n>directly.\n>\n>On Scarlet the persistent names of the cameras are impacted by this and\n>where incorrectly reported as:\n>\n> $ cam -l\n> Available cameras:\n> 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n> 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n>\n>While the expected ones are restored with this fix:\n>\n> $ cam -l\n> Available cameras:\n> 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n> 2: Internal front camera (/base/i2c@ff160000/camera@36)\n>\n>Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n>Signed-off-by: Niklas Söderlund \n>---\n> src/libcamera/sysfs.cpp | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n>diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n>index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n>--- a/src/libcamera/sysfs.cpp\n>+++ b/src/libcamera/sysfs.cpp\n>@@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n> std::string firmwareNodePath(const std::string &device)\n> {\n> \tstd::string fwPath, node;\n>+\tstruct stat st;\n>\n> \t/* Lookup for DT-based systems */\n> \tnode = device + \"/of_node\";\n>-\tif (File::exists(node)) {\n>+\tif (!stat(node.c_str(), &st)) {\n> \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n> \t\tif (!ofPath)\n> \t\t\treturn {};\n>-- \n>2.30.0\n>\n>_______________________________________________\n>libcamera-devel mailing list\n>libcamera-devel@lists.libcamera.org\n>https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A559EC0F1A\n\tfor ;\n\tSat, 9 Jan 2021 06:12:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1588168063;\n\tSat, 9 Jan 2021 07:12:28 +0100 (CET)","from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com\n\t[IPv6:2a00:1450:4864:20::42f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2DF3E6010A\n\tfor ;\n\tSat, 9 Jan 2021 07:12:26 +0100 (CET)","by mail-wr1-x42f.google.com with SMTP id c5so10951067wrp.6\n\tfor ;\n\tFri, 08 Jan 2021 22:12:26 -0800 (PST)","from localhost\n\t(p200300d1ff3099000daa736652ea98b4.dip0.t-ipconnect.de.\n\t[2003:d1:ff30:9900:daa:7366:52ea:98b4])\n\tby smtp.gmail.com with ESMTPSA id\n\tc4sm14626574wmf.19.2021.01.08.22.12.24\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 08 Jan 2021 22:12:25 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"WyBPB/Tk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:reply-to:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=0ZyHM5NDQi/S91buzyLk9m1SAHt/HMw8fWF/nsAGXG0=;\n\tb=WyBPB/TkK0JrfGxtm07bYR0Oiw2aXV5MaB/TlajdI+vfiHENGGKFkQP9YH3RQBZzLM\n\tBOw2cPr2Mn2kPB4Z3wtIlvQTgyW77HIo8BMkKKSPY8Bsu4H3mDRYYfyqbZyrFg6U5Ajf\n\tSOfkNhea/MvIP9Udwtu460S6R3qjfBRFVl+SdqiXBXU/sQRa/ZQYYC9FWp1C8uhqNJG9\n\tS10cDDyKGAGclZQ3uMPyRVWigOtvZ+PM2e1PK4V+OtxTfCPN2quJpFAB1z4i1eu1I8VH\n\tJ1ptFkRKcQ+P4jHG77pCme8p+LmyjNwa08+zHbVZPNmfepDNP9xJepSe7UHa2wws00CQ\n\tz1qg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:reply-to\n\t:references:mime-version:content-disposition\n\t:content-transfer-encoding:in-reply-to;\n\tbh=0ZyHM5NDQi/S91buzyLk9m1SAHt/HMw8fWF/nsAGXG0=;\n\tb=s1aKUs2GCxZVlkATd5GWKGPUPZIHNsMDCeLGAe4Tj8w42xgcHsawB0CW8S9+tq7kua\n\tk+O4Q5bcM8zBZhBr0NYd8QjMQ/7fhqkBMROsT/DRWc34PDrv2RnxqchQZSTig7NYsjOC\n\tnvIJ42jq/BY/KAc5FP0zNBzF87MqGurNhBwFTD3nmbMKRQkZ6/44Q1IOVvUB+euTcUKG\n\tJfeFP8BPpxW6n01quPeSEP8SZ6E0ZDbx9z4rs7DzObqjgClgEUSn5aiRt72uvcuTKNbM\n\t0BH5LJ+vKGgr9TiEV/kTjjaZNEkIhEWvWT2DQWEoCuqo7LxxR1N3n4GmpeQHMTdQOw21\n\tf6uA==","X-Gm-Message-State":"AOAM531R7ZnNGQe5Ie18CXxSz9GTGShWWz6ojQT5UEZ7eq6vWkuXlCvE\n\trHqx/qWjzz6ko7I+t/JV38s=","X-Google-Smtp-Source":"ABdhPJw1Oe0Rim258fmkzzvL0QHXrUvZK/vSoVLpz5R3/kduN2qX/tTnxjsEGhEOJh8PJa5MBBhobw==","X-Received":"by 2002:a5d:4712:: with SMTP id\n\ty18mr6797916wrq.229.1610172745850; \n\tFri, 08 Jan 2021 22:12:25 -0800 (PST)","Date":"Sat, 9 Jan 2021 07:12:23 +0100","From":"Sebastian Fricke ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210109061223.mxq7obfiwad44cwm@basti-TUXEDO-Book-XA1510>","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"Sebastian Fricke ","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14487,"web_url":"https://patchwork.libcamera.org/comment/14487/","msgid":"","date":"2021-01-09T06:35:50","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Sebastian,\n\nThanks for testing patch.\n\nOn 2021-01-09 07:12:23 +0100, Sebastian Fricke wrote:\n> Hey Niklas,\n> \n> Thank you for the patch.\n> I have tested your patch on my FriendlyElec NanoPC-T4 development board\n> with the OV13850.\n> *Before* I applied your patch the output looked like this:\n> ```\n> basti@nanopct4:~$ cam -l\n> Available cameras:\n> 1: Internal front camera (platform/ff110000.i2c/i2c-1/1-0010 ov13850)\n> ```\n> And *after* applying your patch it looks like this:\n> ```\n> basti@nanopct4:~$ cam -l\n> Available cameras:\n> 1: Internal front camera (/base/i2c@ff110000/ov13850@10)\n> ```\n\nIt looks like the patch solves the persistent naming regression on your \nplatform as well. If you test a camera enumeration before \n8f4e16f014c820a0 dose it match the later case \n(/base/i2c@ff110000/ov13850@10) ?\n\n> \n> I also tested this patch on my laptop:\n> *Before*:\n> ```\n> basti@basti-TUXEDO-Book-XA1510:~$ cam -l\n> Available cameras:\n> 1: External camera 'Chicony USB2.0 Camera: Chicony ' (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n> ```\n> *After*:\n> ```\n> basti@basti-TUXEDO-Book-XA1510:~$ cam -l\n> Available cameras:\n> 1: External camera 'Chicony USB2.0 Camera: Chicony ' (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n> ```\n> \n> The output on my laptop looks a little odd, but that is another issue.\n\nWhat is odd about your laptop output?\n\n> \n> Greetings\n> \n> Sebastian\n> \n> On 08.01.2021 18:00, Niklas Söderlund wrote:\n> > The scope of File::exists() was changed to only validate that a file\n> > exists and is therefore not usable to check if a directory exists. This\n> > breaks the persistent name generation for DT based systems as it uses\n> > File::exists() to check for directories, fix this by using stat()\n> > directly.\n> > \n> > On Scarlet the persistent names of the cameras are impacted by this and\n> > where incorrectly reported as:\n> > \n> > $ cam -l\n> > Available cameras:\n> > 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n> > 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n> > \n> > While the expected ones are restored with this fix:\n> > \n> > $ cam -l\n> > Available cameras:\n> > 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n> > 2: Internal front camera (/base/i2c@ff160000/camera@36)\n> > \n> > Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/sysfs.cpp | 3 ++-\n> > 1 file changed, 2 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> > index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n> > --- a/src/libcamera/sysfs.cpp\n> > +++ b/src/libcamera/sysfs.cpp\n> > @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n> > std::string firmwareNodePath(const std::string &device)\n> > {\n> > \tstd::string fwPath, node;\n> > +\tstruct stat st;\n> > \n> > \t/* Lookup for DT-based systems */\n> > \tnode = device + \"/of_node\";\n> > -\tif (File::exists(node)) {\n> > +\tif (!stat(node.c_str(), &st)) {\n> > \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n> > \t\tif (!ofPath)\n> > \t\t\treturn {};\n> > -- \n> > 2.30.0\n> > \n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C6262C3256\n\tfor ;\n\tSat, 9 Jan 2021 06:35:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 466FF68066;\n\tSat, 9 Jan 2021 07:35:54 +0100 (CET)","from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B47266010A\n\tfor ;\n\tSat, 9 Jan 2021 07:35:52 +0100 (CET)","by mail-lf1-x134.google.com with SMTP id s26so28284282lfc.8\n\tfor ;\n\tFri, 08 Jan 2021 22:35:52 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ts27sm2489667ljd.25.2021.01.08.22.35.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 08 Jan 2021 22:35:51 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"0gBcPx/6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=kw7nnAK2HisMKinYFEDkRNgQda3Yd2573WYzUlL03aU=;\n\tb=0gBcPx/6Xe2sr78DFvx1yA73FeueLzNKXw0qPhCzdcxfQ5QRnWviQVB+E3gUi3fvNm\n\t9ebgoR53LJKMGVK7NAxpg2s5zTPT3M679EeptjiR+l3Qk8EQVht2eF5gaZqDDpzQjuey\n\tROLXFWj6lOWEJbGiBJP8ya7FSCABHOe3oyfU/0q1sopFNNjf9DtHRq72IiyiWFkTqYf1\n\tLQIo0SKYMw9Pbg3DbOmyBSmraAa/s+8lEZBsKu//apCD8yH8/XxYgy7QZjh0nWZ9HwJ9\n\tV0r36ZF10gGWF2LfP18AVcYKo6t1a0C2Cq+Fysw1HPjzJAKAewwHQ/6bxmIxqi6G2A5Q\n\t66Xw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=kw7nnAK2HisMKinYFEDkRNgQda3Yd2573WYzUlL03aU=;\n\tb=eOEmSxfRo09y5myrX4NiM72Rjcvz8DYI1XYTPZCXbEsXGvWzbGh4zEpRtZDikX6uA8\n\tGZ/7jBRwF2cKlDycHOEvbhDDlr6bzuVFffbjj2gOy598JxXSg5lL1UX9yeeNwLI2CaN5\n\tihAWzuioNqxq4+yrcoSAxdI3F1Ii/KRxIVVLj36X0JKtOVKNXl26LIZcoghuEOSr/Wn8\n\tGGDpR8HNudvAD8wZhtfUt1MANG1NlKXASMvL+8Sqwfu+jyvp/+mrdRnGd6xsxyIlAoze\n\tQOFsZMq2N2Y0VriuOxru8FybwO7+4PcVrY4akwyPG4BoXXwUn3QXwLRjB/0m1kA6VKeU\n\t4RUQ==","X-Gm-Message-State":"AOAM531rJ6t+G7+FqpvUEhBtpChaemFjRVyUMr8XSvxrQk5cgBCWjvGL\n\tkFlTG/4NtzwSlgeZuxB6r9UANw==","X-Google-Smtp-Source":"ABdhPJwqzJFMQ4SGY5xtl21FgytMJNuFUhvSbc0H5cK7aaweIB/wDCm4qWwd94SgLzEFhE+Id+4FlQ==","X-Received":"by 2002:ac2:5086:: with SMTP id f6mr2912800lfm.593.1610174152018;\n\tFri, 08 Jan 2021 22:35:52 -0800 (PST)","Date":"Sat, 9 Jan 2021 07:35:50 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Sebastian Fricke ","Message-ID":"","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>\n\t<20210109061223.mxq7obfiwad44cwm@basti-TUXEDO-Book-XA1510>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210109061223.mxq7obfiwad44cwm@basti-TUXEDO-Book-XA1510>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14488,"web_url":"https://patchwork.libcamera.org/comment/14488/","msgid":"<20210109082201.xk3lvxaqdz5gwvab@basti-TUXEDO-Book-XA1510>","date":"2021-01-09T08:22:01","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":73,"url":"https://patchwork.libcamera.org/api/people/73/","name":"Sebastian Fricke","email":"sebastian.fricke.linux@gmail.com"},"content":"On 09.01.2021 07:35, Niklas Söderlund wrote:\n>Hi Sebastian,\n>\n>Thanks for testing patch.\n\nIt is my pleasure :)\n\n>\n>On 2021-01-09 07:12:23 +0100, Sebastian Fricke wrote:\n>> Hey Niklas,\n>>\n>> Thank you for the patch.\n>> I have tested your patch on my FriendlyElec NanoPC-T4 development board\n>> with the OV13850.\n>> *Before* I applied your patch the output looked like this:\n>> ```\n>> basti@nanopct4:~$ cam -l\n>> Available cameras:\n>> 1: Internal front camera (platform/ff110000.i2c/i2c-1/1-0010 ov13850)\n>> ```\n>> And *after* applying your patch it looks like this:\n>> ```\n>> basti@nanopct4:~$ cam -l\n>> Available cameras:\n>> 1: Internal front camera (/base/i2c@ff110000/ov13850@10)\n>> ```\n>\n>It looks like the patch solves the persistent naming regression on your\n>platform as well. If you test a camera enumeration before\n>8f4e16f014c820a0 dose it match the later case\n>(/base/i2c@ff110000/ov13850@10) ?\n\nHere the output exactly one patch before '8f4e16f014c820a0':\n\nbasti@nanopct4:~/libcamera$ cam -l\nAvailable cameras:\n1: Internal front camera (/base/i2c@ff110000/ov13850@10)\nbasti@nanopct4:~/libcamera$ git log\ncommit 0ed053245702d193b9c0ad7d8bda9e975ddfb6ed (HEAD -> tmp)\nAuthor: Jacopo Mondi \nDate: Wed Dec 23 18:34:34 2020 +0100\n\n android: camera_device: Simplify properties.get()\n\nAnd here the output when I build with '8f4e16f014c820a0':\n\nbasti@nanopct4:~/libcamera$ cam -l\nAvailable cameras:\n1: Internal front camera (/base/i2c@ff110000/ov13850@10)\nbasti@nanopct4:~/libcamera$ git log\ncommit 8f4e16f014c820a0ecb85e28453b88c277bc859f (HEAD -> tmp)\nAuthor: Kieran Bingham \nDate: Tue Dec 22 13:50:20 2020 +0000\n\n test: file: Check that directories are not treated as files\n\nThe commit you mention does only touch a unit test, so it seems to be\nimpossible that this is the patch that you fix.\n\nI looked it up and the patch that you actually fix is:\n0a823785fad27e1eb9a900e80923bc9bde106de9\n\"libcamera: file: Check file exist()\"\n\nHere is the output with '0a823785fad27e1eb9a900e80923bc9bde106de9'\napplied:\n\nbasti@nanopct4:~/libcamera$ cam -l\nAvailable cameras:\n1: Internal front camera (platform/ff110000.i2c/i2c-1/1-0010 ov13850)\nbasti@nanopct4:~/libcamera$ git log\ncommit 0a823785fad27e1eb9a900e80923bc9bde106de9 (HEAD -> tmp)\nAuthor: Kieran Bingham \nDate: Mon Nov 30 23:25:40 2020 +0000\n\n libcamera: file: Check files exist()\n\nThe patch \"8f4e16f014c820a0ecb85e28453b88c277bc859f\" comes right before\n\"0a823785fad27e1eb9a900e80923bc9bde106de9\". So yes, before '..859f' the\noutput was correct.\n\n>\n>>\n>> I also tested this patch on my laptop:\n>> *Before*:\n>> ```\n>> basti@basti-TUXEDO-Book-XA1510:~$ cam -l\n>> Available cameras:\n>> 1: External camera 'Chicony USB2.0 Camera: Chicony ' (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n>> ```\n>> *After*:\n>> ```\n>> basti@basti-TUXEDO-Book-XA1510:~$ cam -l\n>> Available cameras:\n>> 1: External camera 'Chicony USB2.0 Camera: Chicony ' (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n>> ```\n>>\n>> The output on my laptop looks a little odd, but that is another issue.\n>\n>What is odd about your laptop output?\n\nI mainly think it looks odd because of the starting backslash and the\ntotal different structure. But it is no big issue.\n\n>\n>>\n>> Greetings\n>>\n>> Sebastian\n>>\n>> On 08.01.2021 18:00, Niklas Söderlund wrote:\n>> > The scope of File::exists() was changed to only validate that a file\n>> > exists and is therefore not usable to check if a directory exists. This\n>> > breaks the persistent name generation for DT based systems as it uses\n>> > File::exists() to check for directories, fix this by using stat()\n>> > directly.\n>> >\n>> > On Scarlet the persistent names of the cameras are impacted by this and\n>> > where incorrectly reported as:\n>> >\n>> > $ cam -l\n>> > Available cameras:\n>> > 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n>> > 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n>> >\n>> > While the expected ones are restored with this fix:\n>> >\n>> > $ cam -l\n>> > Available cameras:\n>> > 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n>> > 2: Internal front camera (/base/i2c@ff160000/camera@36)\n>> >\n>> > Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n>> > Signed-off-by: Niklas Söderlund \n>> > ---\n>> > src/libcamera/sysfs.cpp | 3 ++-\n>> > 1 file changed, 2 insertions(+), 1 deletion(-)\n>> >\n>> > diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n>> > index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n>> > --- a/src/libcamera/sysfs.cpp\n>> > +++ b/src/libcamera/sysfs.cpp\n>> > @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n>> > std::string firmwareNodePath(const std::string &device)\n>> > {\n>> > \tstd::string fwPath, node;\n>> > +\tstruct stat st;\n>> >\n>> > \t/* Lookup for DT-based systems */\n>> > \tnode = device + \"/of_node\";\n>> > -\tif (File::exists(node)) {\n>> > +\tif (!stat(node.c_str(), &st)) {\n>> > \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n>> > \t\tif (!ofPath)\n>> > \t\t\treturn {};\n>> > --\n>> > 2.30.0\n>> >\n>> > _______________________________________________\n>> > libcamera-devel mailing list\n>> > libcamera-devel@lists.libcamera.org\n>> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n>-- \n>Regards,\n>Niklas Söderlund","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 35224C0F1A\n\tfor ;\n\tSat, 9 Jan 2021 08:22:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9001268063;\n\tSat, 9 Jan 2021 09:22:04 +0100 (CET)","from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com\n\t[IPv6:2a00:1450:4864:20::32b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F1D260316\n\tfor ;\n\tSat, 9 Jan 2021 09:22:03 +0100 (CET)","by mail-wm1-x32b.google.com with SMTP id n16so7974709wmc.0\n\tfor ;\n\tSat, 09 Jan 2021 00:22:03 -0800 (PST)","from localhost\n\t(p200300d1ff3099000daa736652ea98b4.dip0.t-ipconnect.de.\n\t[2003:d1:ff30:9900:daa:7366:52ea:98b4])\n\tby smtp.gmail.com with ESMTPSA id\n\tv7sm14981868wma.26.2021.01.09.00.22.01\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 09 Jan 2021 00:22:02 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"JHXl7U9/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:reply-to:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=DxFBIXKDIdWCI3UqGjpEAF1uRiDB3uEQSvRPTv4zAD8=;\n\tb=JHXl7U9/f95x4oe0gj5ne1XtHq9Edjz+Zj3XYJanOBBJ8Rqi+PxgFI/RDkfPZGwhwp\n\t0ftPlY7ln38b2na5SQqXPmUjduSPKuz06/Y7WU+gK5Z7/8XmisPHfCKx7ta7w3cvkjbe\n\tRsKFSCta/ykXmbgIWWzrkmO4vMdwU+jJb/izIoXpwcPlPxWv4ebZ6xq9CBxbEtc+cirZ\n\t/7lTa6xykDebee3f+ZS6+BS6VTr6+HGvBOAu3VqZ8blvrYjLezNV4Yn185pAnsFrRT3g\n\thQ+epy0aYN1vdLJK/W7Z4VXnNvRGYPSeX9TFxJFNzrCpvoldtEsOtlc7go76fpMDjy8I\n\t/BlA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:reply-to\n\t:references:mime-version:content-disposition\n\t:content-transfer-encoding:in-reply-to;\n\tbh=DxFBIXKDIdWCI3UqGjpEAF1uRiDB3uEQSvRPTv4zAD8=;\n\tb=dwhCfCcfIzY6BdB3ILELbUNsVz3YrQVVfHeUyID5JLfS5HXOVo7KADJKVmpVzxwTKC\n\tfbcIaS48OVgk2DVatijKsPgiV5SRGOYaYZSFEg1tSFH8YD1+blZYT33jxZ/3UFho9Pvz\n\txgNmS8wpgPcNUm93cghDcTL+FG3W9FUMcve2ADjj1PEP6ecxr/c3BSMr0RmSNhwnRs7N\n\tGjZpx7pw0ACL0grWIXS7lrorqqz8YUsrePH3b6ofgoOePpdyAjvtgkUSo8UfGwpDKkhC\n\tkKxlAdifngG4QTHfD2bjTJLdazfEjZPNNmqReo4bRpV1ME75bhb7XfoZT0wcdci/EbBg\n\tdbsg==","X-Gm-Message-State":"AOAM5339BzVy/kZj7WDhPqfaO1hNOp1CeNQ0PZCr5Unp21NmIiW1/Avk\n\tR+q4HDSo7RSqOKmol7bC+myoCMt91xY=","X-Google-Smtp-Source":"ABdhPJy/9/Guk27tNGlUmBIUerOW/ZaV+9CaAQSTbThuX5k+BRYT3T9/FJZ1arFvPNJVGw/MygPYRg==","X-Received":"by 2002:a1c:48d:: with SMTP id 135mr6381125wme.147.1610180522794;\n\tSat, 09 Jan 2021 00:22:02 -0800 (PST)","Date":"Sat, 9 Jan 2021 09:22:01 +0100","From":"Sebastian Fricke ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210109082201.xk3lvxaqdz5gwvab@basti-TUXEDO-Book-XA1510>","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>\n\t<20210109061223.mxq7obfiwad44cwm@basti-TUXEDO-Book-XA1510>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"Sebastian Fricke ","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14489,"web_url":"https://patchwork.libcamera.org/comment/14489/","msgid":"","date":"2021-01-10T11:04:12","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Sat, Jan 09, 2021 at 03:33:43AM +0100, Niklas Söderlund wrote:\n> On 2021-01-09 03:38:08 +0200, Laurent Pinchart wrote:\n> > On Fri, Jan 08, 2021 at 06:00:42PM +0100, Niklas Söderlund wrote:\n> > > The scope of File::exists() was changed to only validate that a file\n> > > exists and is therefore not usable to check if a directory exists. This\n> > > breaks the persistent name generation for DT based systems as it uses\n> > > File::exists() to check for directories, fix this by using stat()\n> > > directly.\n> > > \n> > > On Scarlet the persistent names of the cameras are impacted by this and\n> > > where incorrectly reported as:\n> > > \n> > > $ cam -l\n> > > Available cameras:\n> > > 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n> > > 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n> > > \n> > > While the expected ones are restored with this fix:\n> > > \n> > > $ cam -l\n> > > Available cameras:\n> > > 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n> > > 2: Internal front camera (/base/i2c@ff160000/camera@36)\n> > > \n> > > Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > src/libcamera/sysfs.cpp | 3 ++-\n> > > 1 file changed, 2 insertions(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> > > index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n> > > --- a/src/libcamera/sysfs.cpp\n> > > +++ b/src/libcamera/sysfs.cpp\n> > > @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n> > > std::string firmwareNodePath(const std::string &device)\n> > > {\n> > > \tstd::string fwPath, node;\n> > > +\tstruct stat st;\n> > > \n> > > \t/* Lookup for DT-based systems */\n> > > \tnode = device + \"/of_node\";\n> > > -\tif (File::exists(node)) {\n> > > +\tif (!stat(node.c_str(), &st)) {\n> > \n> > Hmmm... I think a helper class for this would make sense. I thus wonder\n> > if we shouldn't revert 8f4e16f014c820a0.\n> \n> I'm starting to lean towards that some helpers are not particular \n> helpful :-) I primarily think our File class adds value as it eases \n> usage of map() and unmap(). Wrapping seek(), read() and write() to \n> better cope with C++ data structures is nice but could perhaps be done \n> as streams if it was just that? I think 8f4e16f014c820a0 adds value as \n> it restricts the File class to only work with files.\n\nIt would certainly match the name of the class, and with open()\naccepting directories but mmap(), seek(), read() and write() not being\nable to operate on them, the new behaviour of File::exists() is\ncoherent.\n\n> We could of add a Directory::exists() helper to deal with this. But I \n> think that is going down a bad road as I'm sure someone at some point \n> want to just check if a path exists and don't really care if it's a \n> directory or a file (or symlink). Should we create a helper for each \n> type of lookup or have implementations use multiple helpers at the cost \n> of performance? \n\nA Directory class may be helpful to deal with directories in a better\nway than readdir(), but that's not required now. Something along the\nlines of https://doc.qt.io/qt-5/qfileinfo.html may also make sense at\nsome point.\n\n> I'm think it would be cleaner to use primitives of our environment to \n> deal with things that require no context or cleanup outside of the local \n> scope. It's easier to 'man stat' while reading the code then grepping \n> around in the code base for exactly what File::exists() checks for.\n> \n> That being said I do not feel strongly about this particular patch it \n> just happened to align with my over all feeling for similar situations.\n\nIf you think a FileInfo class would make sense in the future, how about\nadding\n\n\t /* \\todo Replace the stat call with a FileInfo helper class */\n\n? With or without this,\n\nReviewed-by: Laurent Pinchart \n\n> > Kieran, could you elaborate a little bit on the issue you've encountered\n> > that led to that commit being developed ?\n> > \n> > > \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n> > > \t\tif (!ofPath)\n> > > \t\t\treturn {};","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8469EBD808\n\tfor ;\n\tSun, 10 Jan 2021 11:04:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DEDE568063;\n\tSun, 10 Jan 2021 12:04:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 506F260523\n\tfor ;\n\tSun, 10 Jan 2021 12:04:27 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9E8E3DA;\n\tSun, 10 Jan 2021 12:04:26 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mLKkJKX2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610276666;\n\tbh=DyGvE6jQGM2Ynjqxvol49LeprSH7DEOwlvGB3n51cAQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mLKkJKX2eBKCRPMm7YifEvwMb5W+Foig5guCWCyYpNl6+jWLmRLyCx8ouJr62JpkK\n\tSA24D51SePsAYleW2o/p44QKZq1pH31Vy2JGdoOTuHfLfmTC3fRvFqAUsuGEZfPqNb\n\tWUc+THCgtzqtkMSZDnK+RavjhTGGlJuolStnZ3tY=","Date":"Sun, 10 Jan 2021 13:04:12 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14518,"web_url":"https://patchwork.libcamera.org/comment/14518/","msgid":"<92a53daa-fad0-bc6c-9946-1feb11e13a61@ideasonboard.com>","date":"2021-01-11T05:48:20","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 10/01/2021 11:04, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Sat, Jan 09, 2021 at 03:33:43AM +0100, Niklas Söderlund wrote:\n>> On 2021-01-09 03:38:08 +0200, Laurent Pinchart wrote:\n>>> On Fri, Jan 08, 2021 at 06:00:42PM +0100, Niklas Söderlund wrote:\n>>>> The scope of File::exists() was changed to only validate that a file\n>>>> exists and is therefore not usable to check if a directory exists. This\n>>>> breaks the persistent name generation for DT based systems as it uses\n>>>> File::exists() to check for directories, fix this by using stat()\n>>>> directly.\n>>>>\n>>>> On Scarlet the persistent names of the cameras are impacted by this and\n>>>> where incorrectly reported as:\n>>>>\n>>>> $ cam -l\n>>>> Available cameras:\n>>>> 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n>>>> 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n>>>>\n>>>> While the expected ones are restored with this fix:\n>>>>\n>>>> $ cam -l\n>>>> Available cameras:\n>>>> 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n>>>> 2: Internal front camera (/base/i2c@ff160000/camera@36)\n>>>>\n>>>> Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n>>>> Signed-off-by: Niklas Söderlund \n>>>> ---\n>>>> src/libcamera/sysfs.cpp | 3 ++-\n>>>> 1 file changed, 2 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n>>>> index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n>>>> --- a/src/libcamera/sysfs.cpp\n>>>> +++ b/src/libcamera/sysfs.cpp\n>>>> @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n>>>> std::string firmwareNodePath(const std::string &device)\n>>>> {\n>>>> \tstd::string fwPath, node;\n>>>> +\tstruct stat st;\n>>>> \n>>>> \t/* Lookup for DT-based systems */\n>>>> \tnode = device + \"/of_node\";\n>>>> -\tif (File::exists(node)) {\n>>>> +\tif (!stat(node.c_str(), &st)) {\n>>>\n>>> Hmmm... I think a helper class for this would make sense. I thus wonder\n>>> if we shouldn't revert 8f4e16f014c820a0.\n>>\n>> I'm starting to lean towards that some helpers are not particular \n>> helpful :-) I primarily think our File class adds value as it eases \n>> usage of map() and unmap(). Wrapping seek(), read() and write() to \n>> better cope with C++ data structures is nice but could perhaps be done \n>> as streams if it was just that? I think 8f4e16f014c820a0 adds value as \n>> it restricts the File class to only work with files.\n> \n> It would certainly match the name of the class, and with open()\n> accepting directories but mmap(), seek(), read() and write() not being\n> able to operate on them, the new behaviour of File::exists() is\n> coherent.\n\nThat's why I added it this way. File::exists() expects that you could\nthen operate on the target with the other operations of File - which you\ncan't if it's a directory.\n\n\n>> We could of add a Directory::exists() helper to deal with this. But I \n>> think that is going down a bad road as I'm sure someone at some point \n>> want to just check if a path exists and don't really care if it's a \n>> directory or a file (or symlink). Should we create a helper for each \n>> type of lookup or have implementations use multiple helpers at the cost \n>> of performance? \n> \n> A Directory class may be helpful to deal with directories in a better\n> way than readdir(), but that's not required now. Something along the\n> lines of https://doc.qt.io/qt-5/qfileinfo.html may also make sense at\n> some point.\n\nIt feels very much like we just duplicate QT all the time. Should we\njust use QT and call it a dependency?\n\nAt what point do all our helpers simply become 'yet another library'?\n\n>> I'm think it would be cleaner to use primitives of our environment to \n>> deal with things that require no context or cleanup outside of the local \n>> scope. It's easier to 'man stat' while reading the code then grepping \n>> around in the code base for exactly what File::exists() checks for.\n>>\n>> That being said I do not feel strongly about this particular patch it \n>> just happened to align with my over all feeling for similar situations.\n> \n> If you think a FileInfo class would make sense in the future, how about\n> adding\n> \n> \t /* \\todo Replace the stat call with a FileInfo helper class */\n> \n> ? With or without this,\n> \n> Reviewed-by: Laurent Pinchart \n> \n>>> Kieran, could you elaborate a little bit on the issue you've encountered\n>>> that led to that commit being developed ?\n\nI hit an instance where I ensured a file exists before trying to read\nit. (A file, not a directory) - and thus used File::exists() expecting\nit to ... you know, be a file.\n\nOne of the code paths from the ipa proxy passed in a directory, and it\ncrashed. Because the directory existed. That seemed flawed, so I sent\nthe patch to make File::exists check for files, not directories.\n\nWe could make a Directory helper, or change it to use isFile()\nisDirectory() ...\n\n\n>>>> \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n>>>> \t\tif (!ofPath)\n>>>> \t\t\treturn {};","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B4C68BD80C\n\tfor ;\n\tMon, 11 Jan 2021 05:48:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3B32E6809A;\n\tMon, 11 Jan 2021 06:48:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E04B860317\n\tfor ;\n\tMon, 11 Jan 2021 06:48:24 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 25A16EC;\n\tMon, 11 Jan 2021 06:48:23 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hUYmtuDV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610344103;\n\tbh=gsp7jxjtDv3N1zr7fUWgBlolZ7MPcNdFXcXYbevl/z4=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=hUYmtuDV62Tac0UAWMfHGn1vpHsg6parB0GMzHB723g8wKL7f2q96hoekmidQWXfh\n\tjsrRbcPmB/V95WbwyrYoylfIpz8w9uTErf74RgQJDssPxR0zCmBsAyVHa5aIVkOdHY\n\tpwvwDr2GaUiGMpK9XdcfBUIt1aY+BZck7+XojZpE=","To":"Laurent Pinchart , =?utf-8?q?Niklas?=\n\t=?utf-8?q?_S=C3=B6derlund?= ","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>\n\t\n\t\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<92a53daa-fad0-bc6c-9946-1feb11e13a61@ideasonboard.com>","Date":"Mon, 11 Jan 2021 05:48:20 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14519,"web_url":"https://patchwork.libcamera.org/comment/14519/","msgid":"<7c7c03fe-98c1-50e1-d8b8-a7d4fc36fd64@ideasonboard.com>","date":"2021-01-11T05:51:20","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Sebastian,\n\nOn 09/01/2021 08:22, Sebastian Fricke wrote:\n>>\n>>>\n>>> I also tested this patch on my laptop:\n>>> *Before*:\n>>> ```\n>>> basti@basti-TUXEDO-Book-XA1510:~$ cam -l\n>>> Available cameras:\n>>> 1: External camera 'Chicony USB2.0 Camera: Chicony '\n>>> (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n>>> ```\n>>> *After*:\n>>> ```\n>>> basti@basti-TUXEDO-Book-XA1510:~$ cam -l\n>>> Available cameras:\n>>> 1: External camera 'Chicony USB2.0 Camera: Chicony '\n>>> (\\_SB_.PCI0.GP18.XHC0.RHUB.PRT4-4:1.0-04f2:b685)\n>>> ```\n>>>\n>>> The output on my laptop looks a little odd, but that is another issue.\n>>\n>> What is odd about your laptop output?\n> \n> I mainly think it looks odd because of the starting backslash and the\n> total different structure. But it is no big issue.\n\nFor UVC cameras based on ACPI firmware paths, I believe that's expected:\nOn mine:\n\n1: External camera 'HP Wide Vision FHD Camera: HP W'\n(\\_SB_.PCI0.XHC_.RHUB.HS05-5:1.0-0408:5251)\n\n--\nKieran--\nRegards\n--\nKieran","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 64204BD808\n\tfor ;\n\tMon, 11 Jan 2021 05:51:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA86A6809C;\n\tMon, 11 Jan 2021 06:51:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F217960317\n\tfor ;\n\tMon, 11 Jan 2021 06:51:22 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6ED2FEC;\n\tMon, 11 Jan 2021 06:51:22 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qKI5YxaK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610344282;\n\tbh=1TGDAF/UVi/T0WOKGBZsCT7L8Gehn3U1Pt1+uEb2u98=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=qKI5YxaKDR6/xlxhT2ghlaBSm9cw83tHtlegqNRD8Q5azDeS/UF9QF4qva64eMwR0\n\tQdGE/xaJ4wE+g7X5Vh5e0QVMH+bKW5HAtnbKDNccQjmxL8iSTXjDPzVSochQX2XFMS\n\tQe9JxBNkiNCjo+5DmzTP3GOAjZsFTftmbQr99g9I=","To":"Sebastian Fricke , =?utf-8?q?Niklas_?=\n\t=?utf-8?q?S=C3=B6derlund?= ","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>\n\t<20210109061223.mxq7obfiwad44cwm@basti-TUXEDO-Book-XA1510>\n\t\n\t<20210109082201.xk3lvxaqdz5gwvab@basti-TUXEDO-Book-XA1510>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7c7c03fe-98c1-50e1-d8b8-a7d4fc36fd64@ideasonboard.com>","Date":"Mon, 11 Jan 2021 05:51:20 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210109082201.xk3lvxaqdz5gwvab@basti-TUXEDO-Book-XA1510>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14532,"web_url":"https://patchwork.libcamera.org/comment/14532/","msgid":"","date":"2021-01-12T05:13:05","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Jan 11, 2021 at 05:48:20AM +0000, Kieran Bingham wrote:\n> On 10/01/2021 11:04, Laurent Pinchart wrote:\n> > On Sat, Jan 09, 2021 at 03:33:43AM +0100, Niklas Söderlund wrote:\n> >> On 2021-01-09 03:38:08 +0200, Laurent Pinchart wrote:\n> >>> On Fri, Jan 08, 2021 at 06:00:42PM +0100, Niklas Söderlund wrote:\n> >>>> The scope of File::exists() was changed to only validate that a file\n> >>>> exists and is therefore not usable to check if a directory exists. This\n> >>>> breaks the persistent name generation for DT based systems as it uses\n> >>>> File::exists() to check for directories, fix this by using stat()\n> >>>> directly.\n> >>>>\n> >>>> On Scarlet the persistent names of the cameras are impacted by this and\n> >>>> where incorrectly reported as:\n> >>>>\n> >>>> $ cam -l\n> >>>> Available cameras:\n> >>>> 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n> >>>> 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n> >>>>\n> >>>> While the expected ones are restored with this fix:\n> >>>>\n> >>>> $ cam -l\n> >>>> Available cameras:\n> >>>> 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n> >>>> 2: Internal front camera (/base/i2c@ff160000/camera@36)\n> >>>>\n> >>>> Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n> >>>> Signed-off-by: Niklas Söderlund \n> >>>> ---\n> >>>> src/libcamera/sysfs.cpp | 3 ++-\n> >>>> 1 file changed, 2 insertions(+), 1 deletion(-)\n> >>>>\n> >>>> diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> >>>> index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n> >>>> --- a/src/libcamera/sysfs.cpp\n> >>>> +++ b/src/libcamera/sysfs.cpp\n> >>>> @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n> >>>> std::string firmwareNodePath(const std::string &device)\n> >>>> {\n> >>>> \tstd::string fwPath, node;\n> >>>> +\tstruct stat st;\n> >>>> \n> >>>> \t/* Lookup for DT-based systems */\n> >>>> \tnode = device + \"/of_node\";\n> >>>> -\tif (File::exists(node)) {\n> >>>> +\tif (!stat(node.c_str(), &st)) {\n> >>>\n> >>> Hmmm... I think a helper class for this would make sense. I thus wonder\n> >>> if we shouldn't revert 8f4e16f014c820a0.\n> >>\n> >> I'm starting to lean towards that some helpers are not particular \n> >> helpful :-) I primarily think our File class adds value as it eases \n> >> usage of map() and unmap(). Wrapping seek(), read() and write() to \n> >> better cope with C++ data structures is nice but could perhaps be done \n> >> as streams if it was just that? I think 8f4e16f014c820a0 adds value as \n> >> it restricts the File class to only work with files.\n> > \n> > It would certainly match the name of the class, and with open()\n> > accepting directories but mmap(), seek(), read() and write() not being\n> > able to operate on them, the new behaviour of File::exists() is\n> > coherent.\n> \n> That's why I added it this way. File::exists() expects that you could\n> then operate on the target with the other operations of File - which you\n> can't if it's a directory.\n> \n> >> We could of add a Directory::exists() helper to deal with this. But I \n> >> think that is going down a bad road as I'm sure someone at some point \n> >> want to just check if a path exists and don't really care if it's a \n> >> directory or a file (or symlink). Should we create a helper for each \n> >> type of lookup or have implementations use multiple helpers at the cost \n> >> of performance? \n> > \n> > A Directory class may be helpful to deal with directories in a better\n> > way than readdir(), but that's not required now. Something along the\n> > lines of https://doc.qt.io/qt-5/qfileinfo.html may also make sense at\n> > some point.\n> \n> It feels very much like we just duplicate QT all the time. Should we\n> just use QT and call it a dependency?\n\nThat would be a fairly large dependency, although I'd love to see it\nbeing shipped in all Chrome OS and Android devices ;-)\n\n> At what point do all our helpers simply become 'yet another library'?\n\nWe have a bare minimum set of helpers, to accommodate our own needs. We\nmay consider switching to another helper library instead, such as boost\n;-) Jokes aside, I'd love it if there was a small library that offered\nall the helpers we need and nothing else.\n\n> >> I'm think it would be cleaner to use primitives of our environment to \n> >> deal with things that require no context or cleanup outside of the local \n> >> scope. It's easier to 'man stat' while reading the code then grepping \n> >> around in the code base for exactly what File::exists() checks for.\n> >>\n> >> That being said I do not feel strongly about this particular patch it \n> >> just happened to align with my over all feeling for similar situations.\n> > \n> > If you think a FileInfo class would make sense in the future, how about\n> > adding\n> > \n> > \t /* \\todo Replace the stat call with a FileInfo helper class */\n> > \n> > ? With or without this,\n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> >>> Kieran, could you elaborate a little bit on the issue you've encountered\n> >>> that led to that commit being developed ?\n> \n> I hit an instance where I ensured a file exists before trying to read\n> it. (A file, not a directory) - and thus used File::exists() expecting\n> it to ... you know, be a file.\n> \n> One of the code paths from the ipa proxy passed in a directory, and it\n> crashed. Because the directory existed. That seemed flawed, so I sent\n> the patch to make File::exists check for files, not directories.\n\nNote that even if a file exists, you may not have permissions to open\nit, so we shouldn't expect open() to always succeed. Regardless of that,\nthe File::exists() API seems fine as it is today.\n\n> We could make a Directory helper, or change it to use isFile()\n> isDirectory() ...\n> \n> >>>> \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n> >>>> \t\tif (!ofPath)\n> >>>> \t\t\treturn {};","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A5F87BD808\n\tfor ;\n\tTue, 12 Jan 2021 05:13:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2FDA4680C5;\n\tTue, 12 Jan 2021 06:13:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 43FF260523\n\tfor ;\n\tTue, 12 Jan 2021 06:13:20 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A615F3E;\n\tTue, 12 Jan 2021 06:13:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CpNw17Vr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610428399;\n\tbh=W7wmvIbOtXaQ4/ZlpxoudIuRztlCk7SqVkAcXkjeJqU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CpNw17Vr5XiCnA+UN7QyMTqD/jInp0a0mOkuvYU1Q+71aBYZ+0l4z8pdfJ4GgoanV\n\tu06Rq3S4cij+N0Er5kd3iMJ5lcva0bHKKSXOjvxkdBQs6xy+k39hBRz7PJXY70lUm/\n\tjECdSjqor9G66lw/83dxkyLpK2BROQ92uN8XPJno=","Date":"Tue, 12 Jan 2021 07:13:05 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>\n\t\n\t\n\t\n\t<92a53daa-fad0-bc6c-9946-1feb11e13a61@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<92a53daa-fad0-bc6c-9946-1feb11e13a61@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14537,"web_url":"https://patchwork.libcamera.org/comment/14537/","msgid":"<3e4d6c0e-7e5d-5f79-ced3-edde74c3348a@ideasonboard.com>","date":"2021-01-12T09:31:54","subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 08/01/2021 17:00, Niklas Söderlund wrote:\n> The scope of File::exists() was changed to only validate that a file\n> exists and is therefore not usable to check if a directory exists. This\n> breaks the persistent name generation for DT based systems as it uses\n> File::exists() to check for directories, fix this by using stat()\n> directly.\n> \n> On Scarlet the persistent names of the cameras are impacted by this and\n> where incorrectly reported as:\n> \n> $ cam -l\n> Available cameras:\n> 1: Internal front camera (platform/ff160000.i2c/i2c-7/7-003c ov2685)\n> 2: Internal front camera (platform/ff160000.i2c/i2c-7/7-0036 ov5695\n> \n> While the expected ones are restored with this fix:\n> \n> $ cam -l\n> Available cameras:\n> 1: Internal front camera (/base/i2c@ff160000/camera@3c)\n> 2: Internal front camera (/base/i2c@ff160000/camera@36)\n> \n> Fixes: 8f4e16f014c820a0 (\"test: file: Check that directories are not treated as files\")\n> Signed-off-by: Niklas Söderlund \n\nThis certainly returns the existing behaviour so it's a helpful step\nforwards.\n\nWe can consider if we need new helpers later.\n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/libcamera/sysfs.cpp | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> index 3ebe66f8d69b61d4..e9004b2b59c8638d 100644\n> --- a/src/libcamera/sysfs.cpp\n> +++ b/src/libcamera/sysfs.cpp\n> @@ -70,10 +70,11 @@ std::string charDevPath(const std::string &deviceNode)\n> std::string firmwareNodePath(const std::string &device)\n> {\n> \tstd::string fwPath, node;\n> +\tstruct stat st;\n> \n> \t/* Lookup for DT-based systems */\n> \tnode = device + \"/of_node\";\n> -\tif (File::exists(node)) {\n> +\tif (!stat(node.c_str(), &st)) {\n> \t\tchar *ofPath = realpath(node.c_str(), nullptr);\n> \t\tif (!ofPath)\n> \t\t\treturn {};\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 89DB0BD808\n\tfor ;\n\tTue, 12 Jan 2021 09:31:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 183C6680D0;\n\tTue, 12 Jan 2021 10:31:59 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12C0C6054E\n\tfor ;\n\tTue, 12 Jan 2021 10:31:58 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8CE8958B;\n\tTue, 12 Jan 2021 10:31:57 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VF+Ntwyc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610443917;\n\tbh=lBDB9cXgFgY/tChZ06GCGTt5cJudwegXx1miISVHLWM=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=VF+NtwycOXJYteQYQBCYVOtEM3TcKkxzG0ViDVSe1xPHBqXtEOgCOskT5A7veaqL1\n\t5Qs0/eja3rEt0ZP5VlDNmzGxgdN/DybN7UraBQF64uAmosxc8ckIZo2uDvnZiY4Oc9\n\t8GNe5RImtsXn9ZCLRdg0KN6yqybdBeGjl/roTneY=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<3e4d6c0e-7e5d-5f79-ced3-edde74c3348a@ideasonboard.com>","Date":"Tue, 12 Jan 2021 09:31:54 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210108170042.2849407-1-niklas.soderlund@ragnatech.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: sysfs: Fix directory\n\texists check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]