[{"id":21444,"web_url":"https://patchwork.libcamera.org/comment/21444/","msgid":"<163826994626.3059017.2616769931916067665@Monstersaurus>","date":"2021-11-30T10:59:06","subject":"Re: [libcamera-devel] [PATCH v1 2/2] test: Replace \"/proc/self/exe\"\n\twith path to test binary","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-11-30 01:23:03)\n> When tests are run under valgrind, /proc/self/exe points to valgrind,\n> not to the test binary. This results in failures for tests that need to\n> fork processes. Fix it by replacing \"/proc/self/exe\" with the path to\n> the test binary.\n> \n\nThis looks fine to me.\nSomething almost grates on me that self() doesn't feel as obvious to me\nas it should to what it expresses, but it does do the right thing, and I\ndon't think renaming it would help.\n\n(I.e. does self mean the object? the class? is it like 'this'?)\n\nBut ... it's clear enough if the function is read.\n\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Laurent Pinchart \n> ---\n> test/file.cpp | 4 ++--\n> test/ipc/unixsocket.cpp | 5 ++---\n> test/ipc/unixsocket_ipc.cpp | 2 +-\n> test/log/log_process.cpp | 2 +-\n> test/process/process_test.cpp | 2 +-\n> 5 files changed, 7 insertions(+), 8 deletions(-)\n> \n> diff --git a/test/file.cpp b/test/file.cpp\n> index 9ac151c944b5..5c978ebfcada 100644\n> --- a/test/file.cpp\n> +++ b/test/file.cpp\n> @@ -180,7 +180,7 @@ protected:\n> }\n> \n> /* Test size(). */\n> - file.setFileName(\"/proc/self/exe\");\n> + file.setFileName(self());\n> \n> if (file.size() >= 0) {\n> cerr << \"File has valid size before open\" << endl;\n> @@ -277,7 +277,7 @@ protected:\n> file.close();\n> \n> /* Test mapping and unmapping. */\n> - file.setFileName(\"/proc/self/exe\");\n> + file.setFileName(self());\n> file.open(File::OpenModeFlag::ReadOnly);\n> \n> Span data = file.map();\n> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> index 4fc1c10a2125..b3568c0684b0 100644\n> --- a/test/ipc/unixsocket.cpp\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -209,8 +209,7 @@ protected:\n> \n> if (!pid_) {\n> std::string arg = std::to_string(fd);\n> - execl(\"/proc/self/exe\", \"/proc/self/exe\",\n> - arg.c_str(), nullptr);\n> + execl(self().c_str(), self().c_str(), arg.c_str(), nullptr);\n> \n> /* Only get here if exec fails. */\n> exit(TestFail);\n> @@ -464,7 +463,7 @@ private:\n> \n> int prepareFDs(IPCUnixSocket::Payload *message, unsigned int num)\n> {\n> - int fd = open(\"/proc/self/exe\", O_RDONLY);\n> + int fd = open(self().c_str(), O_RDONLY);\n> if (fd < 0)\n> return fd;\n> \n> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> index 2e3b52ca4d4b..1a8d06a12d5c 100644\n> --- a/test/ipc/unixsocket_ipc.cpp\n> +++ b/test/ipc/unixsocket_ipc.cpp\n> @@ -173,7 +173,7 @@ protected:\n> \n> int run()\n> {\n> - ipc_ = std::make_unique(\"\", \"/proc/self/exe\");\n> + ipc_ = std::make_unique(\"\", self().c_str());\n> if (!ipc_->isConnected()) {\n> cerr << \"Failed to create IPCPipe\" << endl;\n> return TestFail;\n> diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> index ca8351335f3a..2484c58f2fa9 100644\n> --- a/test/log/log_process.cpp\n> +++ b/test/log/log_process.cpp\n> @@ -74,7 +74,7 @@ protected:\n> vector args;\n> args.push_back(to_string(exitCode));\n> args.push_back(to_string(num_));\n> - int ret = proc_.start(\"/proc/self/exe\", args);\n> + int ret = proc_.start(self(), args);\n> if (ret) {\n> cerr << \"failed to start process\" << endl;\n> return TestFail;\n> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> index 96bea17f8dce..b410756b3288 100644\n> --- a/test/process/process_test.cpp\n> +++ b/test/process/process_test.cpp\n> @@ -55,7 +55,7 @@ protected:\n> proc_.kill();\n> \n> /* Test starting the process and retrieving the exit code. */\n> - int ret = proc_.start(\"/proc/self/exe\", args);\n> + int ret = proc_.start(self(), args);\n> if (ret) {\n> cerr << \"failed to start process\" << endl;\n> return TestFail;\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 D1CB7BDB13\n\tfor ;\n\tTue, 30 Nov 2021 10:59:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E65D605B4;\n\tTue, 30 Nov 2021 11:59:10 +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 2FB4D60230\n\tfor ;\n\tTue, 30 Nov 2021 11:59:09 +0100 (CET)","from pendragon.ideasonboard.com\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 B80492FD;\n\tTue, 30 Nov 2021 11:59:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MP+KIScm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638269948;\n\tbh=sfLRTw47TYOhve/Nojockm6SEQQWR3Dw/dpM/EGUdRM=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=MP+KIScmwC0AN/H/Zk5AmeIprdnJR7om8/vDd1XKbwh+xJxigyTPdoQAgjs8koloL\n\tQzNVNdBmHoR4tibnyHQ8jyM/8CgAlzmU0dlnQu2P9qew4I2Q8EbBfIA/Fn2TbV2+6C\n\tqWD6YtnbpeR4W/84QRxjim1WpTasWkGtc2yOkwjk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211130012303.10574-3-laurent.pinchart@ideasonboard.com>","References":"<20211130012303.10574-1-laurent.pinchart@ideasonboard.com>\n\t<20211130012303.10574-3-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 30 Nov 2021 10:59:06 +0000","Message-ID":"<163826994626.3059017.2616769931916067665@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 2/2] test: Replace \"/proc/self/exe\"\n\twith path to test binary","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]