[{"id":34023,"web_url":"https://patchwork.libcamera.org/comment/34023/","msgid":"<40899ffd-018e-4a4a-8b33-5509ac8de30f@ideasonboard.com>","date":"2025-04-24T10:23:54","subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2024. 12. 18. 19:27 keltezéssel, Julien Vuillaumier írta:\n> When a child process is started from Process::start(), the file\n> descriptors inherited from the parent process are closed, except\n> the ones explicitly listed in the fds[] argument.\n> \n> One issue is that the file descriptors for stdin, stdout and stderr\n> being closed, the subsequent file descriptors created by the child\n> process will reuse the values 0, 1 and 2 that are now available.\n> Thus usage of printf(), assert() or alike may direct its output\n> to the new resource bound to one of these reused file descriptors.\n> The other issue is that the child process can no longer log on\n> the console because stderr has been closed.\n> \n> To address the 2 issues, Process:start() is amended as below:\n> - Child process stdin and stdout are redirected to /dev/null so\n> that file descriptors 0 and 1 are not reused for any other\n> usage, that could be corrupted by the presence of printf(),\n> assert() or alike.\n> - Child process inherits from parent's stderr in order to share\n> the same logging descriptor.\n> \n> Signed-off-by: Julien Vuillaumier \n> ---\n> src/libcamera/process.cpp | 11 ++++++++++-\n> 1 file changed, 10 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n> index bc9833f4..86a887fb 100644\n> --- a/src/libcamera/process.cpp\n> +++ b/src/libcamera/process.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -259,7 +260,15 @@ int Process::start(const std::string &path,\n> \t\tif (isolate())\n> \t\t\t_exit(EXIT_FAILURE);\n> \n> -\t\tcloseAllFdsExcept(fds);\n> +\t\tstd::vector v(fds);\n> +\t\tv.push_back(STDERR_FILENO);\n> +\t\tcloseAllFdsExcept(v);\n\nI believe the above part is fine.\n\n\n> +\n> +\t\tUniqueFD fd(::open(\"/dev/null\", O_RDWR));\n> +\t\tif (fd.isValid()) {\n> +\t\t\tdup2(fd.get(), STDIN_FILENO);\n> +\t\t\tdup2(fd.get(), STDOUT_FILENO);\n> +\t\t}\n\nBut I am not sure about this. First, it does not handle the case when `STD{IN,OUT}_FILENO`\nis already in `fds`. In that case the file descriptors will be redirected even if that was\nnot the intention. Second, if at least one of `STD{IN,OUT}_FILENO` is not closed in the\nabove `closeAllFdsExcept()` call, then `fd` will be 0 or 1, so the later calls duplicate\na file descriptor into itself, which just looks a bit odd, and might confuse the readers\nof the code. Third, if both `STD{IN,OUT}_FILENO` is in `fds`, then `fd` will be \"leaked\"\ninto the new process since it is never closed if `execv()` succeeds.\n\nI think the following should address both issues (mostly untested):\n\n\n\t\t/* closeAllFdExcept(...) */\n\n\t\tconst auto tryDevNullLowestFd = [](int expected, int oflag) {\n\t\t\tint fd = open(\"/dev/null\", oflag);\n\t\t\tif (fd < 0)\n\t\t\t\texit(EXIT_FAILURE);\n\t\t\tif (fd != expected)\n\t\t\t\tclose(fd);\n\t\t};\n\n\t\ttryDevNullLowestFd(STDIN_FILENO, O_RDONLY);\n\t\ttryDevNullLowestFd(STDOUT_FILENO, O_WRONLY);\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> \t\tconst char *file = utils::secure_getenv(\"LIBCAMERA_LOG_FILE\");\n> \t\tif (file && strcmp(file, \"syslog\"))","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 306B2C327D\n\tfor ;\n\tThu, 24 Apr 2025 10:24:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4142868AC7;\n\tThu, 24 Apr 2025 12:24:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D7B0C68AC5\n\tfor ;\n\tThu, 24 Apr 2025 12:23:57 +0200 (CEST)","from [192.168.33.15] (185.221.143.16.nat.pool.zt.hu\n\t[185.221.143.16])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6D3EF9CE;\n\tThu, 24 Apr 2025 12:23:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dogqisNj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745490235;\n\tbh=HPxsWmR6Pp0hyzD/jVCf2ns7t8Q+qGVDU2OnhxzgQzE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=dogqisNjULUs8amVU0760W90Z2TEfvRYOfLKkGLTPUu9e1S82DcP8T7eLwGbfn5W+\n\tKCHFRpUrWZyfssxIKtJ0HucemaCO1tdjqsDA6FQQZq1L6AfgRi7xmhmZav3QZBLvWY\n\ty4S0lzcOhsWeoHEePaa5VGa+5T1SZ/Rmh7aL7vJU=","Message-ID":"<40899ffd-018e-4a4a-8b33-5509ac8de30f@ideasonboard.com>","Date":"Thu, 24 Apr 2025 12:23:54 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","To":"Julien Vuillaumier ,\n\tlibcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com,\n\tkieran.bingham@ideasonboard.com, paul.elder@ideasonboard.com","References":"<20241218182754.2414920-1-julien.vuillaumier@nxp.com>\n\t<20241218182754.2414920-2-julien.vuillaumier@nxp.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20241218182754.2414920-2-julien.vuillaumier@nxp.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":34260,"web_url":"https://patchwork.libcamera.org/comment/34260/","msgid":"<72896a33-c402-4c0a-a45a-9457eb03eac6@nxp.com>","date":"2025-05-16T14:23:26","subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","submitter":{"id":190,"url":"https://patchwork.libcamera.org/api/people/190/","name":"Julien Vuillaumier","email":"julien.vuillaumier@nxp.com"},"content":"Hello Barnabás,\n\n> \n> 2024. 12. 18. 19:27 keltezéssel, Julien Vuillaumier írta:\n>> When a child process is started from Process::start(), the file\n>> descriptors inherited from the parent process are closed, except\n>> the ones explicitly listed in the fds[] argument.\n>>\n>> One issue is that the file descriptors for stdin, stdout and stderr\n>> being closed, the subsequent file descriptors created by the child\n>> process will reuse the values 0, 1 and 2 that are now available.\n>> Thus usage of printf(), assert() or alike may direct its output\n>> to the new resource bound to one of these reused file descriptors.\n>> The other issue is that the child process can no longer log on\n>> the console because stderr has been closed.\n>>\n>> To address the 2 issues, Process:start() is amended as below:\n>> - Child process stdin and stdout are redirected to /dev/null so\n>> that file descriptors 0 and 1 are not reused for any other\n>> usage, that could be corrupted by the presence of printf(),\n>> assert() or alike.\n>> - Child process inherits from parent's stderr in order to share\n>> the same logging descriptor.\n>>\n>> Signed-off-by: Julien Vuillaumier \n>> ---\n>>   src/libcamera/process.cpp | 11 ++++++++++-\n>>   1 file changed, 10 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n>> index bc9833f4..86a887fb 100644\n>> --- a/src/libcamera/process.cpp\n>> +++ b/src/libcamera/process.cpp\n>> @@ -12,6 +12,7 @@\n>>   #include \n>>   #include \n>>   #include \n>> +#include \n>>   #include \n>>   #include \n>>   #include \n>> @@ -259,7 +260,15 @@ int Process::start(const std::string &path,\n>>               if (isolate())\n>>                       _exit(EXIT_FAILURE);\n>>\n>> -             closeAllFdsExcept(fds);\n>> +             std::vector v(fds);\n>> +             v.push_back(STDERR_FILENO);\n>> +             closeAllFdsExcept(v);\n> \n> I believe the above part is fine.\n> \n> \n>> +\n>> +             UniqueFD fd(::open(\"/dev/null\", O_RDWR));\n>> +             if (fd.isValid()) {\n>> +                     dup2(fd.get(), STDIN_FILENO);\n>> +                     dup2(fd.get(), STDOUT_FILENO);\n>> +             }\n> \n> But I am not sure about this. First, it does not handle the case when \n> `STD{IN,OUT}_FILENO`\n> is already in `fds`. In that case the file descriptors will be \n> redirected even if that was\n> not the intention. Second, if at least one of `STD{IN,OUT}_FILENO` is \n> not closed in the\n> above `closeAllFdsExcept()` call, then `fd` will be 0 or 1, so the later \n> calls duplicate\n> a file descriptor into itself, which just looks a bit odd, and might \n> confuse the readers\n> of the code. Third, if both `STD{IN,OUT}_FILENO` is in `fds`, then `fd` \n> will be \"leaked\"\n> into the new process since it is never closed if `execv()` succeeds.\n\nAbout your first point, intent was to unconditionally redirect \nstdin/stdout file descriptors to the null device. But letting the option \nto the parent process to share the stdin/stdout descriptors with its \nchild makes the implementation more generic indeed.\n\nI agree with your other points: the possible occurence of descriptor \nself duplication is awkward. Also, the null device file descriptor \nremains unnecessarily opened.\n\n> \n> I think the following should address both issues (mostly untested):\n> \n> \n>                /* closeAllFdExcept(...) */\n> \n>                const auto tryDevNullLowestFd = [](int expected, int \n> oflag) {\n>                        int fd = open(\"/dev/null\", oflag);\n>                        if (fd < 0)\n>                                exit(EXIT_FAILURE);\n>                        if (fd != expected)\n>                                close(fd);\n>                };\n> \n>                tryDevNullLowestFd(STDIN_FILENO, O_RDONLY);\n>                tryDevNullLowestFd(STDOUT_FILENO, O_WRONLY);\n\nHaving tested your proposal, it works fine and addresses above issues. \nAlso, to support the (unlikely?) case where stderr could be closed by \nthe parent process, statement below may be added to make sure fd=2 is \nalways bound to something in the child process:\n\n\ttryDevNullLowestFd(STDERR_FILENO, O_WRONLY);\n\n\nThus, I'll update with a V3 based on your proposal. As the resulting \npatch will be mostly based on your code, do you want to be mentioned \nwith some tag in the commit message?\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n>>\n>>               const char *file = \n>> utils::secure_getenv(\"LIBCAMERA_LOG_FILE\");\n>>               if (file && strcmp(file, \"syslog\"))\n> \n\nThanks,\nJulien","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 99F32C3200\n\tfor ;\n\tFri, 16 May 2025 14:23:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A2B2C68C92;\n\tFri, 16 May 2025 16:23:32 +0200 (CEST)","from EUR03-VI1-obe.outbound.protection.outlook.com\n\t(mail-vi1eur03on2061b.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:260c::61b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DF7568B66\n\tfor ;\n\tFri, 16 May 2025 16:23:31 +0200 (CEST)","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)\n\tby DU2PR04MB8870.eurprd04.prod.outlook.com (2603:10a6:10:2e1::17)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8722.31;\n\tFri, 16 May 2025 14:23:29 +0000","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a]) by\n\tAM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a%7]) with mapi id 15.20.8722.027;\n\tFri, 16 May 2025 14:23:29 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"mvrgowuR\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=IOdIlQu3e5yqeDabwNY1jqARb0k104VP7t1Q6jSbgEJCjdKDF8S35hxs1AeAAOaHtYQQJEd3Q2dG71lwzA6WJtL9w+8YNA5zJ464aTE0zgH0MYZ0e7nG46X88kwG3PDNkzjtULlFE92p+BWwU8diCgpXR2ABkJ9Hwpx5GoVwkUEdSwR9/Lh6zh3LjVvo2/amVbSHiB6rgv57KbD0wBwOW8F/+M3WUrTI0vN+ETE2+eqOJVeWaKbGrF00MobAmyaI5CL1huoh3Rt6WA78ofXc3kGajHt8TPQEEB1+CWsbKzvY6FZ6G6rLH3iZVnouY5eK0ELjeBVg0dWSU0nxfqHCcA==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=Lv+UsRx/VHrPMR8HFCD1e2t6jg39BQfVVS99Mps6nl4=;\n\tb=dWXTD9KpWrixoQ5cLIueLets9jcSxtk0CvWpnXY5xjSe/JoUHhFlRGl/G4kXVYFmYsdKFvLzgXib91zo/H5wzLYMTP+iIdXzfHiglFEExLeCGep5SawzR+WhRmUlwh3Hbh1I8LcZZeQYIQwxYei7pu45VZTCqa0bVw0GWJcUsqy563f3VBLngwZEsymTbhun1lQwCBBq/odEOdAan4VQo5Znqwg/q2+4PwqcxdEOxZzoY0URgIfCuPUf4NXCWyhpquBQAWGmRwJYpu6w0ySyTlgIeRbPZgvqNYvGVLkophzUcZF2F0EBLavYEVEWrROZUKiokP92ZYQRAoF/4hS1lg==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=Lv+UsRx/VHrPMR8HFCD1e2t6jg39BQfVVS99Mps6nl4=;\n\tb=mvrgowuRX1l3VXdSWlJ0gi3IPd4+M12h7nvt18ekDOfnCMy1zB353HcjacIgv2NMGt4FQslxS9t8oa1gearPSo6wHn4YoPj3U139ur7enaR51GysI/li3ThnC8wuz0wB5g6nDqv/4ten3jSGEpV45V23EyJCBtppToIdW5FEcpSFiye86vBXinII9pPVBFm3f4cSfaHhhqIFuj7LmJWYQsLjyWRxQxIcChU6Fo7dJ/sxiu0QHhbJjE/xudiQvcSxrv1R7IB1XtO1xtJCPgnWinNgmshabvVBFaLygBGHB3hmsRe2ZSdRNldNWknMkn/YObQkrfQzm+rxbaTy9LhTJw==","Message-ID":"<72896a33-c402-4c0a-a45a-9457eb03eac6@nxp.com>","Date":"Fri, 16 May 2025 16:23:26 +0200","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com, \n\tKieran Bingham\n\t, paul.elder@ideasonboard.com","References":"<20241218182754.2414920-1-julien.vuillaumier@nxp.com>\n\t<20241218182754.2414920-2-julien.vuillaumier@nxp.com>\n\t<40899ffd-018e-4a4a-8b33-5509ac8de30f@ideasonboard.com>","Content-Language":"en-US","From":"Julien Vuillaumier ","In-Reply-To":"<40899ffd-018e-4a4a-8b33-5509ac8de30f@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-ClientProxiedBy":"AM0PR02CA0116.eurprd02.prod.outlook.com\n\t(2603:10a6:20b:28c::13) To AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"AM9PR04MB8147:EE_|DU2PR04MB8870:EE_","X-MS-Office365-Filtering-Correlation-Id":"3b3fb7d7-0501-4add-fe33-08dd94853a0b","X-LD-Processed":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635,ExtAddr","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;ARA:13230040|1800799024|376014|366016;","X-Microsoft-Antispam-Message-Info":"=?utf-8?q?gvg9myaHY+geK92ahjoBF4zccRtr?=\n\t=?utf-8?q?QIi1A6Iwj7E4s956lMsZ3myG2mgdkzK/rcLFKyOF9ErWMlP788eE8pWc?=\n\t=?utf-8?q?PRtXj8DF5MzBkAQIGNZQ58+EmkjLh7YI0UbzWuriBte0GkxlXQhgaPS9?=\n\t=?utf-8?q?RbPUD5sDaXy2VtcYKnyxHDk5d5mPiYfgsQNtVaPD9braHWF8FiFFOA+y?=\n\t=?utf-8?q?Qo4koMjRz1yGPWN6iIbbrvWpQmGFWt+FbwvuPnp4DQAbHcnbPq3dOhTk?=\n\t=?utf-8?q?45tz2uz5RaI+IgzLKrN+ErxvCyBE9kYnPT2U5ynvKxCWtBUdqaHL5cLa?=\n\t=?utf-8?q?3ZcpGvePzJOJO9sYLu8bqGSJ8nO5sWEWK10O3CV6+NSo7nIYDpJ+hwtZ?=\n\t=?utf-8?q?c5HE9pmK1cEbfoJJvsmtTSGJeTcYh2DFkA48GlrbkEKCzurTX4ZFwBKv?=\n\t=?utf-8?q?4KUzow/PBpg4z8FRflsw52kvhYIUpWJ1VqEGthNymJH9ejE+Pm5KU6XZ?=\n\t=?utf-8?q?GXFf3DCzFQCZ5bHrjEQIZTISlz1ln7Jykp/r51BMIXwbiSKujBlHLSn+?=\n\t=?utf-8?q?qs6liym0lKyUwSJyGIyMiI1JSGGyWPEm+RlKw7A8JfPYCEhYfGb9udXT?=\n\t=?utf-8?q?0Nigt/B8TB6cZ8xNPoCRtUFXwcuuAYloPehzynie33XmMtRkKcqFS0Hm?=\n\t=?utf-8?q?xVheUuABFPepCwki6ChOu29yAVGKSV4gaF4R1TZU5KFc3O1KNU7eBbUH?=\n\t=?utf-8?q?CkRHxYYUWwAC7aFeDxiSwGsU02A+q746ijsNhIMxQprrqFoG45/EZD5A?=\n\t=?utf-8?q?Mn3guNAFTjTDPwha/cvkjUL8UBEJo7CAR3Xq+/6eHEOZJbnnX41w/oH/?=\n\t=?utf-8?q?Yz+vyLa8DSSpowAFF8eFYzPcKOvsXPnbMxi3KLohRR/1lsowfZgK/RKr?=\n\t=?utf-8?q?1hpwMXEADPYyB6z4FW6nTrmJoucnJyJQFMrKWKTQkbs5o2zBvc3AHA5v?=\n\t=?utf-8?q?+n+nBVEHMkPCg0W+0vjr0bzrClvveytieyEClQZ9pASKOuQSREgq39L+?=\n\t=?utf-8?q?L4fX8OshVoLt6BBBi9cJdFkyNmhk6Y8veyQvaG1sSmUFeJCUsMJQYCsh?=\n\t=?utf-8?q?nOZNzzYUIMwo+jXa4Dg4U6l1CKAJ5mhmKUxBRwhQtjt67Yr9WWLxyYZI?=\n\t=?utf-8?q?N2dc6U2p+6H1yBM2pD8MMDipXFLveMOVf+e53quqQjY2ejNmrUeZVr9d?=\n\t=?utf-8?q?GI0Blp6EmYEAnBrQBAWV5Ep7mdRafJzBAOiR+Z5rb6daaLBi1SOcYCa8?=\n\t=?utf-8?q?e5iG4lhfLI8wT3E5TZFTbWIpUNNw/hupHj+VHrItan9GtD7f80DNYRBw?=\n\t=?utf-8?q?ciLAC+O+oc5LKxn/xXiwW18Hx9FtmkCMAsaMAjJ6I50tNfdm4NxeQ8t/?=\n\t=?utf-8?q?Zsadke5QWH6KUFtmYZJ2W8f1Ou6B1oteKn7BbSNIwB89U4ZbJS+NR6o8?=\n\t=?utf-8?q?w4idhKq0I8LvZYyHSNUDrML1uOCNlccKJsyeOrRrEc4HxLeOb/TSiWBF?=\n\t=?utf-8?q?2jy20NAMqRrk5fS/ExqA1fk=3D?=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:AM9PR04MB8147.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230040)(1800799024)(376014)(366016); DIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?xwMueokHHaYTymId6i1T/uwex?=\n\t=?utf-8?q?jbiA79JkuHq5054gjXkFUH0MlWqmgV0mhx2ZId4GBmdf35XzrMWURTG/?=\n\t=?utf-8?q?vB2yW0jrXgrcL46CTBpkER4SIbMz15Z0FpGypL3xB0Y4M4aatDKrKccL?=\n\t=?utf-8?q?3mmdQjlqSECKf7gqwEyia3fVXsLzaAWKmblyjRiCKv5Q72MHS4/E05Er?=\n\t=?utf-8?q?7cUFreTSF6U9nJBE3slD9sfA9FUuDAqmhj6jND6fKnmFM3djAqbCeb+e?=\n\t=?utf-8?q?2sutIVEHOgaG9w456AT1LkvfkDvL8KOyWlka6BqCDuFc5DbWgYg67bXd?=\n\t=?utf-8?q?9SqHjyuJGObtyDcdrQWNR7QNZcYOZnpDtIvVY31Pi5R6StJ+ieRhL/TP?=\n\t=?utf-8?q?X+QXFW2Uf310a+lVoev897fCTVO4SowZrPqaEGezJCv0P/UGfwNnFZY6?=\n\t=?utf-8?q?S1HJAativw5QxbcfKgECPXa8yOobeelQg7k2tU5egO0wVfdn5+VtzHNS?=\n\t=?utf-8?q?MEPBJXRchoWbHIEprMaodRMtNvkpf90DdsrzYir30v2F2sPPqRFPvGAb?=\n\t=?utf-8?q?zgwISxXw0gVlyHCybzXy7Hto8LVz1dRRvSEKZSRo2a0wt3UaIuzmQV4c?=\n\t=?utf-8?q?KTTYC13+/mRoZ8avzXrdf5NX2+3ojnPqffwO61f6EmPmfXJQqBhLeWqj?=\n\t=?utf-8?q?IgpPoQ12mykUKn8TC5YNtVuDXS8aELigqn4c0++1bttU0sWKQzb7bVpP?=\n\t=?utf-8?q?3Y5ILCwnVYFj2A3zTvzlSIgCCYdAT+7pPP4MnDTqQHcmPAYHHFoSaKVF?=\n\t=?utf-8?q?WIyygSmUuSCrnmLNJAg4EQbsysKtYrchm4rwFnMMNzLJ7oU0DEHZdfEZ?=\n\t=?utf-8?q?nSei4vB+FIE58EdDpLqmcPfPOJhRX+MkfUO27pjAl8nwdzoQUQBC7bW4?=\n\t=?utf-8?q?Mi3oVa/uWjK0B9HSBbbp2ioYsOagRXd1zSSyrQXdomazQ1aUFyez1HmA?=\n\t=?utf-8?q?9rBucIuYXNf/w/vYo2EG7XXwdh9Kgl/CePxmFVc3+VgTvAke3usvg3fP?=\n\t=?utf-8?q?uIwnsv2zTBDDs64IUKjpxwmX7rJlSRB8XeMUGpsjKHfPIpzNNlgltMa0?=\n\t=?utf-8?q?VZUbrtmOUptF4bLqoKRAnJI7ZAufF0MwB6M5ltSID+XXAVosBPzNNxAW?=\n\t=?utf-8?q?/MYPeql2izT9t2jxVH0yWn7Caj3aCUaim+56Yv+0HFuyXPkrkv5GdOte?=\n\t=?utf-8?q?Og3a3NrbAImpPb8uVDXBeVKVRbe79bkvs2BizbcAOxBAnEU+t9Towfe3?=\n\t=?utf-8?q?CeV8EGSdnVOxTslteJKQi616ZlU7TBZABesU3dvkMIktYPZhadOCjOeo?=\n\t=?utf-8?q?rzUdk/H8YsiCjcyiWmtmaV2EKd/zTSEi+xZ7UACItu7gI7ypUJ56iihu?=\n\t=?utf-8?q?K/wv8mR0o1iOJuDikYI/tk2GPCSnDiJcwEcmzSWudYakgTBChmt8Mhox?=\n\t=?utf-8?q?WzCMemtwSuPPdGlNijAu4fS0A5o9q3to04Z1VwsM6UWKEGtn3FyX787g?=\n\t=?utf-8?q?GzLkowrk2XukmrDKBDiuWfmNgcuO3q+0LIXsU3odkX6nmczFfcCC0XYk?=\n\t=?utf-8?q?u+oWZ7fZfle7BXghzRVgEwV9T+rNYO5ZzSka16EeZtjvl1OQLkmmYMw9?=\n\t=?utf-8?q?RY5ha5rixbkfx7RZwwy4Y2m1RIg6vlOFJozJ6kIeEaOqiLb7NfbEpdb3?=\n\t=?utf-8?q?j9xajtM5bua7kQEPG2yAaN19RimnQ=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"3b3fb7d7-0501-4add-fe33-08dd94853a0b","X-MS-Exchange-CrossTenant-AuthSource":"AM9PR04MB8147.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"16 May 2025 14:23:29.4165\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"5RRx68uEyX9zuX3Hg3EaoW3hIV7Jr2o26VLxRLbKIk2xnYgAlGry7cUadYlQpc4QFIsqC7uSa2coXhdTHUp3BzodQfZk7D+/Mlhwfvqdvno=","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"DU2PR04MB8870","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\" "}},{"id":34262,"web_url":"https://patchwork.libcamera.org/comment/34262/","msgid":"","date":"2025-05-16T15:26:44","subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 05. 16. 16:23 keltezéssel, Julien Vuillaumier írta:\n> Hello Barnabás,\n> \n>>\n>> 2024. 12. 18. 19:27 keltezéssel, Julien Vuillaumier írta:\n>>> When a child process is started from Process::start(), the file\n>>> descriptors inherited from the parent process are closed, except\n>>> the ones explicitly listed in the fds[] argument.\n>>>\n>>> One issue is that the file descriptors for stdin, stdout and stderr\n>>> being closed, the subsequent file descriptors created by the child\n>>> process will reuse the values 0, 1 and 2 that are now available.\n>>> Thus usage of printf(), assert() or alike may direct its output\n>>> to the new resource bound to one of these reused file descriptors.\n>>> The other issue is that the child process can no longer log on\n>>> the console because stderr has been closed.\n>>>\n>>> To address the 2 issues, Process:start() is amended as below:\n>>> - Child process stdin and stdout are redirected to /dev/null so\n>>> that file descriptors 0 and 1 are not reused for any other\n>>> usage, that could be corrupted by the presence of printf(),\n>>> assert() or alike.\n>>> - Child process inherits from parent's stderr in order to share\n>>> the same logging descriptor.\n>>>\n>>> Signed-off-by: Julien Vuillaumier \n>>> ---\n>>>   src/libcamera/process.cpp | 11 ++++++++++-\n>>>   1 file changed, 10 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n>>> index bc9833f4..86a887fb 100644\n>>> --- a/src/libcamera/process.cpp\n>>> +++ b/src/libcamera/process.cpp\n>>> @@ -12,6 +12,7 @@\n>>>   #include \n>>>   #include \n>>>   #include \n>>> +#include \n>>>   #include \n>>>   #include \n>>>   #include \n>>> @@ -259,7 +260,15 @@ int Process::start(const std::string &path,\n>>>               if (isolate())\n>>>                       _exit(EXIT_FAILURE);\n>>>\n>>> -             closeAllFdsExcept(fds);\n>>> +             std::vector v(fds);\n>>> +             v.push_back(STDERR_FILENO);\n>>> +             closeAllFdsExcept(v);\n>>\n>> I believe the above part is fine.\n>>\n>>\n>>> +\n>>> +             UniqueFD fd(::open(\"/dev/null\", O_RDWR));\n>>> +             if (fd.isValid()) {\n>>> +                     dup2(fd.get(), STDIN_FILENO);\n>>> +                     dup2(fd.get(), STDOUT_FILENO);\n>>> +             }\n>>\n>> But I am not sure about this. First, it does not handle the case when `STD{IN,OUT}_FILENO`\n>> is already in `fds`. In that case the file descriptors will be redirected even if that was\n>> not the intention. Second, if at least one of `STD{IN,OUT}_FILENO` is not closed in the\n>> above `closeAllFdsExcept()` call, then `fd` will be 0 or 1, so the later calls duplicate\n>> a file descriptor into itself, which just looks a bit odd, and might confuse the readers\n>> of the code. Third, if both `STD{IN,OUT}_FILENO` is in `fds`, then `fd` will be \"leaked\"\n>> into the new process since it is never closed if `execv()` succeeds.\n> \n> About your first point, intent was to unconditionally redirect stdin/stdout file descriptors to the null device. But letting the option to the parent process to share the stdin/stdout descriptors with its child makes the implementation more generic indeed.\n> \n> I agree with your other points: the possible occurence of descriptor self duplication is awkward. Also, the null device file descriptor remains unnecessarily opened.\n> \n>>\n>> I think the following should address both issues (mostly untested):\n>>\n>>\n>>                 /* closeAllFdExcept(...) */\n>>\n>>                 const auto tryDevNullLowestFd = [](int expected, int oflag) {\n>>                         int fd = open(\"/dev/null\", oflag);\n>>                         if (fd < 0)\n>>                                 exit(EXIT_FAILURE);\n>>                         if (fd != expected)\n>>                                 close(fd);\n>>                 };\n>>\n>>                 tryDevNullLowestFd(STDIN_FILENO, O_RDONLY);\n>>                 tryDevNullLowestFd(STDOUT_FILENO, O_WRONLY);\n> \n> Having tested your proposal, it works fine and addresses above issues. Also, to support the (unlikely?) case where stderr could be closed by the parent process, statement below may be added to make sure fd=2 is always bound to something in the child process:\n> \n>     tryDevNullLowestFd(STDERR_FILENO, O_WRONLY);\n\nI have a version of this change locally, and in that I also added this,\nso I agree that this should be part of it.\n\nI am wondering if maybe the `fd < 0` condition should be extended with\n`&& errno != ENFILE && errno != EMFILE`.\n\n\n> \n> \n> Thus, I'll update with a V3 based on your proposal. As the resulting patch will be mostly based on your code, do you want to be mentioned with some tag in the commit message?\n\nNo, it's fine.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>>\n>>>               const char *file = utils::secure_getenv(\"LIBCAMERA_LOG_FILE\");\n>>>               if (file && strcmp(file, \"syslog\"))\n>>\n> \n> Thanks,\n> Julien","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 1A6B4C3200\n\tfor ;\n\tFri, 16 May 2025 15:26:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E56C68C92;\n\tFri, 16 May 2025 17:26:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FB7F68B66\n\tfor ;\n\tFri, 16 May 2025 17:26:49 +0200 (CEST)","from [192.168.33.17] (185.221.142.248.nat.pool.zt.hu\n\t[185.221.142.248])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 154134AD;\n\tFri, 16 May 2025 17:26:31 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"spuxgq1o\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747409191;\n\tbh=TT4eWmeH7Ed00DcDuo7pAlV+P0D5GBtulZTARvedv6M=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=spuxgq1ogkxR6M+b7RhJaL59C4DVDV6ZCnOPRxrXEsct0OVwcNnmWUfdkiVFAv7lG\n\thhrWHmp5xxY2GOHffO/1xXolY/mxRRSgAGqNRhGJFRO+8Zh89jR7uNcQ07raR2QKnr\n\tlGd4hskNAd9WaXjEnU/c5GjJIis1Cq4ph16S3Y8A=","Message-ID":"","Date":"Fri, 16 May 2025 17:26:44 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","To":"Julien Vuillaumier ,\n\tlibcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com,\n\tKieran Bingham ,\n\tpaul.elder@ideasonboard.com","References":"<20241218182754.2414920-1-julien.vuillaumier@nxp.com>\n\t<20241218182754.2414920-2-julien.vuillaumier@nxp.com>\n\t<40899ffd-018e-4a4a-8b33-5509ac8de30f@ideasonboard.com>\n\t<72896a33-c402-4c0a-a45a-9457eb03eac6@nxp.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<72896a33-c402-4c0a-a45a-9457eb03eac6@nxp.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":34268,"web_url":"https://patchwork.libcamera.org/comment/34268/","msgid":"","date":"2025-05-19T08:30:54","subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","submitter":{"id":190,"url":"https://patchwork.libcamera.org/api/people/190/","name":"Julien Vuillaumier","email":"julien.vuillaumier@nxp.com"},"content":"Hello Barnabás,\n\nOn 16/05/2025 17:26, Barnabás Pőcze wrote:\n> \n> Hi\n> \n> 2025. 05. 16. 16:23 keltezéssel, Julien Vuillaumier írta:\n>> Hello Barnabás,\n>>\n>>>\n>>> 2024. 12. 18. 19:27 keltezéssel, Julien Vuillaumier írta:\n>>>> When a child process is started from Process::start(), the file\n>>>> descriptors inherited from the parent process are closed, except\n>>>> the ones explicitly listed in the fds[] argument.\n>>>>\n>>>> One issue is that the file descriptors for stdin, stdout and stderr\n>>>> being closed, the subsequent file descriptors created by the child\n>>>> process will reuse the values 0, 1 and 2 that are now available.\n>>>> Thus usage of printf(), assert() or alike may direct its output\n>>>> to the new resource bound to one of these reused file descriptors.\n>>>> The other issue is that the child process can no longer log on\n>>>> the console because stderr has been closed.\n>>>>\n>>>> To address the 2 issues, Process:start() is amended as below:\n>>>> - Child process stdin and stdout are redirected to /dev/null so\n>>>> that file descriptors 0 and 1 are not reused for any other\n>>>> usage, that could be corrupted by the presence of printf(),\n>>>> assert() or alike.\n>>>> - Child process inherits from parent's stderr in order to share\n>>>> the same logging descriptor.\n>>>>\n>>>> Signed-off-by: Julien Vuillaumier \n>>>> ---\n>>>>   src/libcamera/process.cpp | 11 ++++++++++-\n>>>>   1 file changed, 10 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n>>>> index bc9833f4..86a887fb 100644\n>>>> --- a/src/libcamera/process.cpp\n>>>> +++ b/src/libcamera/process.cpp\n>>>> @@ -12,6 +12,7 @@\n>>>>   #include \n>>>>   #include \n>>>>   #include \n>>>> +#include \n>>>>   #include \n>>>>   #include \n>>>>   #include \n>>>> @@ -259,7 +260,15 @@ int Process::start(const std::string &path,\n>>>>               if (isolate())\n>>>>                       _exit(EXIT_FAILURE);\n>>>>\n>>>> -             closeAllFdsExcept(fds);\n>>>> +             std::vector v(fds);\n>>>> +             v.push_back(STDERR_FILENO);\n>>>> +             closeAllFdsExcept(v);\n>>>\n>>> I believe the above part is fine.\n>>>\n>>>\n>>>> +\n>>>> +             UniqueFD fd(::open(\"/dev/null\", O_RDWR));\n>>>> +             if (fd.isValid()) {\n>>>> +                     dup2(fd.get(), STDIN_FILENO);\n>>>> +                     dup2(fd.get(), STDOUT_FILENO);\n>>>> +             }\n>>>\n>>> But I am not sure about this. First, it does not handle the case when \n>>> `STD{IN,OUT}_FILENO`\n>>> is already in `fds`. In that case the file descriptors will be \n>>> redirected even if that was\n>>> not the intention. Second, if at least one of `STD{IN,OUT}_FILENO` is \n>>> not closed in the\n>>> above `closeAllFdsExcept()` call, then `fd` will be 0 or 1, so the \n>>> later calls duplicate\n>>> a file descriptor into itself, which just looks a bit odd, and might \n>>> confuse the readers\n>>> of the code. Third, if both `STD{IN,OUT}_FILENO` is in `fds`, then \n>>> `fd` will be \"leaked\"\n>>> into the new process since it is never closed if `execv()` succeeds.\n>>\n>> About your first point, intent was to unconditionally redirect \n>> stdin/stdout file descriptors to the null device. But letting the \n>> option to the parent process to share the stdin/stdout descriptors \n>> with its child makes the implementation more generic indeed.\n>>\n>> I agree with your other points: the possible occurence of descriptor \n>> self duplication is awkward. Also, the null device file descriptor \n>> remains unnecessarily opened.\n>>\n>>>\n>>> I think the following should address both issues (mostly untested):\n>>>\n>>>\n>>>                 /* closeAllFdExcept(...) */\n>>>\n>>>                 const auto tryDevNullLowestFd = [](int expected, int \n>>> oflag) {\n>>>                         int fd = open(\"/dev/null\", oflag);\n>>>                         if (fd < 0)\n>>>                                 exit(EXIT_FAILURE);\n>>>                         if (fd != expected)\n>>>                                 close(fd);\n>>>                 };\n>>>\n>>>                 tryDevNullLowestFd(STDIN_FILENO, O_RDONLY);\n>>>                 tryDevNullLowestFd(STDOUT_FILENO, O_WRONLY);\n>>\n>> Having tested your proposal, it works fine and addresses above issues. \n>> Also, to support the (unlikely?) case where stderr could be closed by \n>> the parent process, statement below may be added to make sure fd=2 is \n>> always bound to something in the child process:\n>>\n>>      tryDevNullLowestFd(STDERR_FILENO, O_WRONLY);\n> \n> I have a version of this change locally, and in that I also added this,\n> so I agree that this should be part of it.\n\nThen I will add the STDERR_FILENO step.\n\n> I am wondering if maybe the `fd < 0` condition should be extended with\n> `&& errno != ENFILE && errno != EMFILE`.\n\nA new process running out of file descriptors suggests there is \nsomething wrong in the system - aborting with exit() can be a reasonable \noption here. In the present case of the isolated IPA, it typically would \nnot be able to open a calibration file anyway.\n\nThis addition may cause issues in case of ENFILE/EMFILE occurence:\n- invalid file descriptor will be close() which can be confusing\n- the logic of sequentially testing in order stdin/stdout/stderr no \nlonger works reliably: ENFILE could be returned during stdin step, \nmeaning that fd=0 would remain free - later, during the stdout step, \nnull device open() may succeed and bound to fd=0 ; then the comparison \nlogic with the target fd=1 will fail, and both fd=0 and fd=1 will remain \nfree.\n\nSo it seems the child process could run with free file descriptors fd=0 \nor fd=1 which is risky. Therefore, I would suggest not adding this \nENFILE / EMFILE test.\n\nThanks,\nJulien\n\n>>\n>>\n>> Thus, I'll update with a V3 based on your proposal. As the resulting \n>> patch will be mostly based on your code, do you want to be mentioned \n>> with some tag in the commit message?\n> \n> No, it's fine.\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n>>\n>>>\n>>>\n>>> Regards,\n>>> Barnabás Pőcze\n>>>\n>>>>\n>>>>               const char *file = \n>>>> utils::secure_getenv(\"LIBCAMERA_LOG_FILE\");\n>>>>               if (file && strcmp(file, \"syslog\"))\n>>>\n>>\n>> Thanks,\n>> Julien\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 AAE98C31E9\n\tfor ;\n\tMon, 19 May 2025 08:31:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 95F0C68D7A;\n\tMon, 19 May 2025 10:30:59 +0200 (CEST)","from MRWPR03CU001.outbound.protection.outlook.com\n\t(mail-francesouthazlp170110003.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:c207::3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C15B616A3\n\tfor ;\n\tMon, 19 May 2025 10:30:58 +0200 (CEST)","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)\n\tby DU4PR04MB11032.eurprd04.prod.outlook.com (2603:10a6:10:58a::10)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8746.30;\n\tMon, 19 May 2025 08:30:56 +0000","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a]) by\n\tAM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a%7]) with mapi id 15.20.8722.027;\n\tMon, 19 May 2025 08:30:56 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"RYqCGTre\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=fWvcd5U3VVvawd6b36yEg69cJD3I1B3X1CGXhdcYVImezDOKTRmbbZFuPllRor/6BJVRZuNnOAoiR6gJJAPSX0HH74h11L07ggOMco4wsxKxuR0X4RfAcomZKwYYP+LMiUvCcvWWiA3zqoukWqCqmM5vVPr5OnWFbqeO7MFZ9IVIHv0VhlBqaf4ty3/owv6i4D1TBoPJSRrB13VHTSbSxmHWyaJkqlez0wD+Z2uMtbm+D6196hUoPgcpiFNhwxGlYE+P4Z0+RZ8eTG+B+Bom5Ke2IujwbXfJDG9yDDcMyx3VPZ34r6bbeSqeIDlbvUCQsRsL9iEVwUOsQt7aVdVo6A==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=Q+l0rRjmUgsogxLGxtlVka5cLzBfpugJ+b3p7tJgV/o=;\n\tb=wSlN8d/5U/K3To+eTEIJEwzatwLpiRMXQAZgYjnryhTHRaAVH/ZApdQmG1pMu6yDLcM4hgDn0atvtVeHFM8A+0TPp6IptRY+WdXTGXxJdG3EwB/WH4XFVaYkz6nKM05kf4NhtuFhq+y3QEPd6O/L/Kd9M0w8y5ABjhVsosId5xwXCMghe3s6wLi4YDTDfXC6gji0f+9geVfoQY391cndspdX542CCYOOwxyrHVQ7X9Yolx033dt/LReaEpP4vds7tsOhvWUpdhOX5WNQe6P9nfH5NUDfh8QsllhL0pO4j11EHLBQ0rEt8BLluMCjeVlv9GFKZ8Bz7Lm2i/hBVMSZKg==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=Q+l0rRjmUgsogxLGxtlVka5cLzBfpugJ+b3p7tJgV/o=;\n\tb=RYqCGTre3nrfVND+SXzGJyedzxJZNPa+IwlgDP3mXM+uNWdOThokEmL1LtdBSTIWeXgpMtwXjqWFi4ApcyelNAuaN1QulmArgApxlv1ZmTJrhkjGnDU2ZtOrAdPuRAwI8sReEfoGfvGU/0+dGH+8ie9m+K4uT0d/Ysfp+UC8NbJGoJ0F2bXS53SJstDs5nrX0vvgbSBPz+PdKv11oGY9lD0+bHG9xQ0pzJeuZXG42b6dZU6q2Bt1SbZtIVwPxtKfjA6XNxOEYLT2LoZqmCKgUTgGFi2CkycE+XzotWrsoWWJfyl6Y4sMJ25Jk3riFgjK2X5257qcAE2zxEb3nIgbGw==","Message-ID":"","Date":"Mon, 19 May 2025 10:30:54 +0200","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com, \n\tKieran Bingham\n\t, paul.elder@ideasonboard.com","References":"<20241218182754.2414920-1-julien.vuillaumier@nxp.com>\n\t<20241218182754.2414920-2-julien.vuillaumier@nxp.com>\n\t<40899ffd-018e-4a4a-8b33-5509ac8de30f@ideasonboard.com>\n\t<72896a33-c402-4c0a-a45a-9457eb03eac6@nxp.com>\n\t","Content-Language":"en-US","From":"Julien Vuillaumier ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-ClientProxiedBy":"AS4P191CA0008.EURP191.PROD.OUTLOOK.COM\n\t(2603:10a6:20b:5d5::19) To AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"AM9PR04MB8147:EE_|DU4PR04MB11032:EE_","X-MS-Office365-Filtering-Correlation-Id":"1ca2cf46-248c-4aeb-9831-08dd96af797f","X-LD-Processed":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635,ExtAddr","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;ARA:13230040|376014|1800799024|366016;","X-Microsoft-Antispam-Message-Info":"=?utf-8?q?r7a6TsrBa6jlIJj0xKOrxWuxYGjA?=\n\t=?utf-8?q?tVIOu4sksIMHuiy4/BXKW8F1pLhyxcN9viszfJYExjLa/y6A09Mrw9sz?=\n\t=?utf-8?q?aRAUqf6RBS80VHWCxLDUXVPmkENGt0uUJ1K0XjUnS6py21mCJSsRI6XC?=\n\t=?utf-8?q?/NOxHHcrSmjCz2jNGzzve3VnruNKQ2MAOvM49En03AKhSztWElvuXjrg?=\n\t=?utf-8?q?9DJnoK/mrzps6lI0B2ZGPddWTqI69Hji8Skn0QqxBn6vPFgSDi6C3i/O?=\n\t=?utf-8?q?h9mfu9vFREh4fGifLecS4sa4hWlT4LC4pQQdrMAF165WS+CfYN9w8uqh?=\n\t=?utf-8?q?a6uBvD0vTySFY2w5LqSofb0YJKUAaKm0ViAHTwlwMMg/Cw/c78dS1gfc?=\n\t=?utf-8?q?4MZ02GPgxZPO1UMA2cPXd6rVduHgCn33RjZQEqOBrUHmIXEhQDa7q28F?=\n\t=?utf-8?q?JvCRsAkchPdZFzCFtPW59PjDgrFvzLHciad0YOtvpjNvdbs3UHwCxe9F?=\n\t=?utf-8?q?TazcbIGBo1G9X4s3j72qdkoRGc2MEhtNuyhe8PVR1qfI3Znkl8MqjCgM?=\n\t=?utf-8?q?EOOmyViQOkCUhRGbqpmUd8BODYx0r4VsKwYIDfRigCWhQZtxet0hs0Fp?=\n\t=?utf-8?q?LIwJKOnUnfULjnydGdwdFx2yUCUkKQTL1KaHL5C0knGUpj+d/ARemGcW?=\n\t=?utf-8?q?PNH68Y8oTWNP395bALMwYKd9aReIZjRZDBkAzfy7pcF9Qn4JdilwOZqp?=\n\t=?utf-8?q?ubu5DNvSBzA5kM+fPWvGO2+39QN6amI1Lt9qYjauDosaTdj2nWQg8gxU?=\n\t=?utf-8?q?9pCfJ9jCAhwytUo4x0FiUf0yEkF3HthYOTTNq1Y/w0l3pdzSMhq1WGDn?=\n\t=?utf-8?q?V9eMDKL5NXS62ZHmn/IwBqZ1oAkYCaXsw/5u78wOTfpMSom7VkXqB+Rt?=\n\t=?utf-8?q?cIPO1W80krqt0FlEd6PIXsAvugpJAD0Tx0sLx5shSS0g5g1/gDBCnMqO?=\n\t=?utf-8?q?L6FWF9M20qIh6aDWwu+4aL75a8kuuaKtNqjqHjpKdtZx5e1kNzhpcijD?=\n\t=?utf-8?q?HYIabj0bmma2pF4s19tPkE79sjhPCZt1sdIpRlag3wAsfCN318EagVdv?=\n\t=?utf-8?q?XdyUyrHiJaIzZuYx33k5m4K9+hMyROfk68su0vINtW5D3cAOMN6FgOow?=\n\t=?utf-8?q?8TBDl7uJnrxplMoUmZvbqdp4kDPWCeGkv7xOdTdHFYNUyPe2T+oEk9je?=\n\t=?utf-8?q?la91lnonex7fDlyuAFDq0zf4efeAoyjcwIUn0v9vsWUS3nOAnSJxrQH+?=\n\t=?utf-8?q?7dD6sRbEPcojr5xgVxN/L3O+vwZW6AzA1cOcSxwfcv6gVE1m41LeZyXR?=\n\t=?utf-8?q?BS1VvfovS+3v+CiIH/Eon2B9V4LPnS3WBn9GrZHAGbrCt4bzuXdx91+S?=\n\t=?utf-8?q?CNShdyNJG/22PgUyYfwEn2h0MVCIvX0zobrmX2RmtDdHb9PN6lsqgQZa?=\n\t=?utf-8?q?cz3ov6nUn4hb3XcvROCJZ/650GwQ4YFtGUuuY7QsOxxS14heKK1vLbVb?=\n\t=?utf-8?q?5S/n8slS/lBjuJAFb1jpS5w=3D?=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:AM9PR04MB8147.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230040)(376014)(1800799024)(366016); DIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?aUtMYcHphJxcPQ17EgERro/6/?=\n\t=?utf-8?q?LKbGI7GQnBVYGedWQ/wvwEi+9ItXzFLHcZ7zHQlBX30s0UJ0wytDxNWL?=\n\t=?utf-8?q?DqzfVXrs9MYDKpTOV9WBma+v68uhO2rTzuu9tUx32hy6WCfp9tbVQT76?=\n\t=?utf-8?q?tuLW7/yw64ZsphVfhpG2FeEAm2LEH4q3y+kM+RV+5qKSD2MOD5/j4uF+?=\n\t=?utf-8?q?Z5TYOH4Gtay8UNA701BaiF9EJV92DRsaGNBmx0ldQNSUHf04bnjRi+vV?=\n\t=?utf-8?q?vwdNyD0Apv8U0ITeERPi5iXOOeYYaztE8kCHuDpHQk120ttFwsMday5V?=\n\t=?utf-8?q?HDRhEkkgmgpcOqB5dbnAHhuN/jK+Hin/MmtUvUzTbtl5pAP5erMFDtk+?=\n\t=?utf-8?q?pgFoY1ozZBmHFCUgCTeLZsUjbbknfhMSGQrv5XbJj/rMVYCbV+35BcX7?=\n\t=?utf-8?q?IIe3FyDpFpF/kD0Mwle6Jna3rH9Fx2qYAQKQeHGKkBEhquHCmefZoP/W?=\n\t=?utf-8?q?xq83wcgh9a34L4Z7YE0oU70O+mOzE1yROnnBt/dWupqth8+9sUB27Gpc?=\n\t=?utf-8?q?U30lP66Zl01dabWMXf0Qi2OkA3RNcw9UiWKEiGXuP1A2QRsUsxAtd857?=\n\t=?utf-8?q?8m3vFqvyUTr0Eccms5M3Xp3Spk/SOE4FXE7tR5HJJyT0p6p4spUDJhfl?=\n\t=?utf-8?q?0UZAQFh9w9Cynw84jHWtUi0B/eAlvEMc/jMNbnXdVRcKpidY9UkGbBHC?=\n\t=?utf-8?q?bqK9NKAjDZGdt7rStRA08ydNzdljZh4m9rX1qDowWwyEgH/72yquQK2u?=\n\t=?utf-8?q?qdAFV3527fMVDv0uVRTXaithEwwSEORyoLNQgE5v2gUrM2jOr8QCR35S?=\n\t=?utf-8?q?/vti0YlhXuqamb/W3++/ykDOnzMyAbklmdlivCsRbJ94+FxANYE8cAJ8?=\n\t=?utf-8?q?QB7wZctv3rPuF8Xs6j0VCkmMju8TfzvWtTDf/P4SqCa7k9b9LFJCBQKY?=\n\t=?utf-8?q?8EipluqKpS3pNlo7avbtDfHaMSKWJPQzBo6d1uV7tkK0MpVAN/rXrJhr?=\n\t=?utf-8?q?P7cXefRnoo9brqW0AEoBj1+EXqJjyfHsiescHqEvEsvKMXvOAPMB70th?=\n\t=?utf-8?q?KHvGm+CqeJaehXipaHwZtnmPk+OzzP4+K9APB++b1+4umZmN2pvdUdal?=\n\t=?utf-8?q?ux1WTcEC1tW3MTUVTAyWCRdBfbLY5oLZeIYJlSkL8u5nLaefonPrJy6x?=\n\t=?utf-8?q?PhDtrfFr6iDuyx6BgAM1b0ZAGGiYFxc7i8BBdxIxc9Jo/F7vPeaNAmS5?=\n\t=?utf-8?q?ACQYO9Gna8J/Bd+x2BqMPgIdHzkWZb/jDZ75JoXudhcVnVTwQhRP7br+?=\n\t=?utf-8?q?t/HF6CaSanGeP8XAVY/DD92PHoQERNrme8X4PB1G/NENy2Mc8AHrPNFY?=\n\t=?utf-8?q?OHZQuLbhGJg+vDp5J4j5L1+ibxeTnPf47MU4IsqS257PzjYH9WZ9GAoS?=\n\t=?utf-8?q?IS18d2BhXN/BbrgMNzgtfFmUfVY0I/EMgfGp4H0wdld5yunDY4guD+qj?=\n\t=?utf-8?q?1MgvapH+MBnXrOCl5lrK8ljl41ajtG115u7PF/DzaQ1iUnpkZzFUDlrP?=\n\t=?utf-8?q?BrBneFVGXDAdla0qirKjq0ek0YvFnn8d1c2a/m4hbuO5pvl6xmn/2nH6?=\n\t=?utf-8?q?CtGaQiaX6x2xneW14JQt3uybbw199sFMkG5E5FqsXpQxmQvwe0gkVemw?=\n\t=?utf-8?q?ut+GrgJqyQO/vm9sxfp9I8kmmMSyA=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"1ca2cf46-248c-4aeb-9831-08dd96af797f","X-MS-Exchange-CrossTenant-AuthSource":"AM9PR04MB8147.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"19 May 2025 08:30:56.1734\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"X3asN2q44huTurd1AmBIxB8FDFfib+5tFdDcqzp7NugX04WVvC0zspbBpetclz3TuL13QFzNVzb+LCRp+loI1X4rAyGQuwAklNEQY2xV2z8=","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"DU4PR04MB11032","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\" "}},{"id":34270,"web_url":"https://patchwork.libcamera.org/comment/34270/","msgid":"","date":"2025-05-19T08:59:58","subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 05. 19. 10:30 keltezéssel, Julien Vuillaumier írta:\n> Hello Barnabás,\n> \n> On 16/05/2025 17:26, Barnabás Pőcze wrote:\n>>\n>> Hi\n>>\n>> 2025. 05. 16. 16:23 keltezéssel, Julien Vuillaumier írta:\n>>> Hello Barnabás,\n>>>\n>>>>\n>>>> 2024. 12. 18. 19:27 keltezéssel, Julien Vuillaumier írta:\n>>>>> When a child process is started from Process::start(), the file\n>>>>> descriptors inherited from the parent process are closed, except\n>>>>> the ones explicitly listed in the fds[] argument.\n>>>>>\n>>>>> One issue is that the file descriptors for stdin, stdout and stderr\n>>>>> being closed, the subsequent file descriptors created by the child\n>>>>> process will reuse the values 0, 1 and 2 that are now available.\n>>>>> Thus usage of printf(), assert() or alike may direct its output\n>>>>> to the new resource bound to one of these reused file descriptors.\n>>>>> The other issue is that the child process can no longer log on\n>>>>> the console because stderr has been closed.\n>>>>>\n>>>>> To address the 2 issues, Process:start() is amended as below:\n>>>>> - Child process stdin and stdout are redirected to /dev/null so\n>>>>> that file descriptors 0 and 1 are not reused for any other\n>>>>> usage, that could be corrupted by the presence of printf(),\n>>>>> assert() or alike.\n>>>>> - Child process inherits from parent's stderr in order to share\n>>>>> the same logging descriptor.\n>>>>>\n>>>>> Signed-off-by: Julien Vuillaumier \n>>>>> ---\n>>>>>   src/libcamera/process.cpp | 11 ++++++++++-\n>>>>>   1 file changed, 10 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n>>>>> index bc9833f4..86a887fb 100644\n>>>>> --- a/src/libcamera/process.cpp\n>>>>> +++ b/src/libcamera/process.cpp\n>>>>> @@ -12,6 +12,7 @@\n>>>>>   #include \n>>>>>   #include \n>>>>>   #include \n>>>>> +#include \n>>>>>   #include \n>>>>>   #include \n>>>>>   #include \n>>>>> @@ -259,7 +260,15 @@ int Process::start(const std::string &path,\n>>>>>               if (isolate())\n>>>>>                       _exit(EXIT_FAILURE);\n>>>>>\n>>>>> -             closeAllFdsExcept(fds);\n>>>>> +             std::vector v(fds);\n>>>>> +             v.push_back(STDERR_FILENO);\n>>>>> +             closeAllFdsExcept(v);\n>>>>\n>>>> I believe the above part is fine.\n>>>>\n>>>>\n>>>>> +\n>>>>> +             UniqueFD fd(::open(\"/dev/null\", O_RDWR));\n>>>>> +             if (fd.isValid()) {\n>>>>> +                     dup2(fd.get(), STDIN_FILENO);\n>>>>> +                     dup2(fd.get(), STDOUT_FILENO);\n>>>>> +             }\n>>>>\n>>>> But I am not sure about this. First, it does not handle the case when `STD{IN,OUT}_FILENO`\n>>>> is already in `fds`. In that case the file descriptors will be redirected even if that was\n>>>> not the intention. Second, if at least one of `STD{IN,OUT}_FILENO` is not closed in the\n>>>> above `closeAllFdsExcept()` call, then `fd` will be 0 or 1, so the later calls duplicate\n>>>> a file descriptor into itself, which just looks a bit odd, and might confuse the readers\n>>>> of the code. Third, if both `STD{IN,OUT}_FILENO` is in `fds`, then `fd` will be \"leaked\"\n>>>> into the new process since it is never closed if `execv()` succeeds.\n>>>\n>>> About your first point, intent was to unconditionally redirect stdin/stdout file descriptors to the null device. But letting the option to the parent process to share the stdin/stdout descriptors with its child makes the implementation more generic indeed.\n>>>\n>>> I agree with your other points: the possible occurence of descriptor self duplication is awkward. Also, the null device file descriptor remains unnecessarily opened.\n>>>\n>>>>\n>>>> I think the following should address both issues (mostly untested):\n>>>>\n>>>>\n>>>>                 /* closeAllFdExcept(...) */\n>>>>\n>>>>                 const auto tryDevNullLowestFd = [](int expected, int oflag) {\n>>>>                         int fd = open(\"/dev/null\", oflag);\n>>>>                         if (fd < 0)\n>>>>                                 exit(EXIT_FAILURE);\n>>>>                         if (fd != expected)\n>>>>                                 close(fd);\n>>>>                 };\n>>>>\n>>>>                 tryDevNullLowestFd(STDIN_FILENO, O_RDONLY);\n>>>>                 tryDevNullLowestFd(STDOUT_FILENO, O_WRONLY);\n>>>\n>>> Having tested your proposal, it works fine and addresses above issues. Also, to support the (unlikely?) case where stderr could be closed by the parent process, statement below may be added to make sure fd=2 is always bound to something in the child process:\n>>>\n>>>      tryDevNullLowestFd(STDERR_FILENO, O_WRONLY);\n>>\n>> I have a version of this change locally, and in that I also added this,\n>> so I agree that this should be part of it.\n> \n> Then I will add the STDERR_FILENO step.\n> \n>> I am wondering if maybe the `fd < 0` condition should be extended with\n>> `&& errno != ENFILE && errno != EMFILE`.\n> \n> A new process running out of file descriptors suggests there is something wrong in the system - aborting with exit() can be a reasonable option here. In the present case of the isolated IPA, it typically would not be able to open a calibration file anyway.\n> \n> This addition may cause issues in case of ENFILE/EMFILE occurence:\n> - invalid file descriptor will be close() which can be confusing\n\nThat's true, but I don't think it is a big issue.\n\n> - the logic of sequentially testing in order stdin/stdout/stderr no longer works reliably: ENFILE could be returned during stdin step, meaning that fd=0 would remain free - later, during the stdout step, null device open() may succeed and bound to fd=0 ; then the comparison logic with the target fd=1 will fail, and both fd=0 and fd=1 will remain free.\n\nAhh, you're right about ENFILE. Let's just exit when open() goes wrong for any reason.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> So it seems the child process could run with free file descriptors fd=0 or fd=1 which is risky. Therefore, I would suggest not adding this ENFILE / EMFILE test.\n> \n> Thanks,\n> Julien\n> \n>>>\n>>>\n>>> Thus, I'll update with a V3 based on your proposal. As the resulting patch will be mostly based on your code, do you want to be mentioned with some tag in the commit message?\n>>\n>> No, it's fine.\n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>>\n>>>>\n>>>>\n>>>> Regards,\n>>>> Barnabás Pőcze\n>>>>\n>>>>>\n>>>>>               const char *file = utils::secure_getenv(\"LIBCAMERA_LOG_FILE\");\n>>>>>               if (file && strcmp(file, \"syslog\"))\n>>>>\n>>>\n>>> Thanks,\n>>> Julien\n>>\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 9D0B0C31E9\n\tfor ;\n\tMon, 19 May 2025 09:00:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42D4E68D7A;\n\tMon, 19 May 2025 11:00:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 534F7616A3\n\tfor ;\n\tMon, 19 May 2025 11:00:02 +0200 (CEST)","from [192.168.33.12] (185.221.142.248.nat.pool.zt.hu\n\t[185.221.142.248])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3DCCB496;\n\tMon, 19 May 2025 10:59:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Q6E8EXp8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747645182;\n\tbh=7v6rjWfYZKM97WlpqHmrkh0utQCU5YNSdgMSNFNK3Kg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=Q6E8EXp8LTHSP9XrZDsPxDJVz3HMylyHsBU0r627Yl+Q9YXKdLbS+xmt8o5JA0nTs\n\tJqXU6rECkmMr9lWBLQ4kqpf05umc1FRAG8if2F3FXPWlxdBxgxaCsht8jcgDBihnwG\n\tcKm61CvjQlDmRv+EsvreBd4Ws8Mt9BIXJIVX79vw=","Message-ID":"","Date":"Mon, 19 May 2025 10:59:58 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/1] libcamera: process: Pass stderr and reserve stdin\n\tand stdout fds","To":"Julien Vuillaumier ,\n\tlibcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com,\n\tKieran Bingham ,\n\tpaul.elder@ideasonboard.com","References":"<20241218182754.2414920-1-julien.vuillaumier@nxp.com>\n\t<20241218182754.2414920-2-julien.vuillaumier@nxp.com>\n\t<40899ffd-018e-4a4a-8b33-5509ac8de30f@ideasonboard.com>\n\t<72896a33-c402-4c0a-a45a-9457eb03eac6@nxp.com>\n\t\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]