[{"id":3260,"web_url":"https://patchwork.libcamera.org/comment/3260/","msgid":"<20191215154316.GF4889@pendragon.ideasonboard.com>","date":"2019-12-15T15:43:16","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: v4l2_device,\n\tv4l2_videodevice: call open system call directly","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sun, Dec 08, 2019 at 11:56:02PM -0500, Paul Elder wrote:\n> We are preparing to integrate the V4L2 adaptation layer, which will\n> intercept open() calls (among others) via LD_PRELOAD. To prevent\n> libcamera's own open() calls from being intercepted, replace them with a\n> direct syscall.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nWe may want to later use syscall() for more of our calls in order to\nimprove performances. I'm in particular thinking about ioctl(). We\nshould then provide a helper to do so, but for now, I think using\nsyscall() directly is fine.\n\n> ---\n> No change in v2\n> ---\n>  src/libcamera/v4l2_device.cpp      | 3 ++-\n>  src/libcamera/v4l2_videodevice.cpp | 3 ++-\n>  2 files changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 0452f801..f16931a3 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -11,6 +11,7 @@\n>  #include <iomanip>\n>  #include <string.h>\n>  #include <sys/ioctl.h>\n> +#include <sys/syscall.h>\n>  #include <unistd.h>\n>  \n>  #include \"log.h\"\n> @@ -75,7 +76,7 @@ int V4L2Device::open(unsigned int flags)\n>  \t\treturn -EBUSY;\n>  \t}\n>  \n> -\tint ret = ::open(deviceNode_.c_str(), flags);\n> +\tint ret = syscall(SYS_open, deviceNode_.c_str(), flags);\n\nSYS_open is deprecated (but still supported of course) in favour of\nSYS_openat. Should we already switch to\n\n\tint ret = syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), flags);\n\n? glibc does so. I'm also wondering if we should set O_LARGEFILE\nunconditionally, but that seems to need some more research.\n\n>  \tif (ret < 0) {\n>  \t\tret = -errno;\n>  \t\tLOG(V4L2, Error) << \"Failed to open V4L2 device: \"\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 99213075..2dff3d3c 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -13,6 +13,7 @@\n>  #include <string.h>\n>  #include <sys/ioctl.h>\n>  #include <sys/mman.h>\n> +#include <sys/syscall.h>\n>  #include <sys/time.h>\n>  #include <unistd.h>\n>  #include <vector>\n> @@ -1427,7 +1428,7 @@ int V4L2M2MDevice::open()\n>  \t * as the V4L2VideoDevice::open() retains a handle by duplicating the\n>  \t * fd passed in.\n>  \t */\n> -\tfd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK);\n> +\tfd = syscall(SYS_open, deviceNode_.c_str(), O_RDWR | O_NONBLOCK);\n\nSame here. With this addressed (or not if you think it's not needed),\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  \tif (fd < 0) {\n>  \t\tret = -errno;\n>  \t\tLOG(V4L2, Error)","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 68C0B60100\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Dec 2019 16:43:26 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D19AFDD;\n\tSun, 15 Dec 2019 16:43:25 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1576424606;\n\tbh=NR2BQ2QxI0ysN2hU7ke3Cp3wcfc7dWR7QB1JTcl7GaQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RbT9CoO7aton+kyhmdIXGmMW/P/HnqYpJ25E5uvqGzrf8exnROB3D51kCDjEUqvny\n\t+9uVuKuiX9QpeqK/RwAjfZMfokz7Hw7pkuh3ELYNrhQtfTMOfnW6V0M3W10ZoJ/yte\n\tIbBlIrGiED8PkD/NCgI7FLR8vgsLewFTqadUff80=","Date":"Sun, 15 Dec 2019 17:43:16 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191215154316.GF4889@pendragon.ideasonboard.com>","References":"<20191209045603.6245-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191209045603.6245-1-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: v4l2_device,\n\tv4l2_videodevice: call open system call directly","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 15 Dec 2019 15:43:26 -0000"}}]