[{"id":29623,"web_url":"https://patchwork.libcamera.org/comment/29623/","msgid":"<171681685939.668709.5360171995976012742@ping.linuxembedded.co.uk>","date":"2024-05-27T13:34:19","subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2024-05-27 14:24:29)\n> GCC 14 thinks `rects` is a \"possibly dangling reference to a temporary\":\n> \n> /libcamera/src/android/camera_capabilities.cpp: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n> /libcamera/src/android/camera_capabilities.cpp:1084:46: error: possibly dangling reference to a temporary [-Werror=dangling-reference]\n> 1084 | const Span& rects =\n> | ^~~~~\n> /libcamera/src/android/camera_capabilities.cpp:1085:83: note: the temporary was destroyed at the end of the full expression ‘(& properties)->libcamera::ControlList::get >(libcamera::properties::PixelArrayActiveAreas).std::optional >::value_or >(libcamera::Span())’\n> 1085 | properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~\n> \n> The return value of `value_or()` is indeed a temporary, but\n> binding it to a reference extends its lifetime. Avoid the\n> warning by not using a reference; this does not make much\n> difference since `value_or()` does not return a reference.\n\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/android/camera_capabilities.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 6f4d48de..71043e12 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1081,7 +1081,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> }\n> \n> {\n> - const Span &rects =\n> + const Span rects =\n> properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> std::vector data{\n> static_cast(rects[0].x),\n> -- \n> 2.45.1\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 C9097BD87C\n\tfor ;\n\tMon, 27 May 2024 13:34:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04CA9634B1;\n\tMon, 27 May 2024 15:34:24 +0200 (CEST)","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 50866634AD\n\tfor ;\n\tMon, 27 May 2024 15:34:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE827183;\n\tMon, 27 May 2024 15:34:19 +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=\"sLF+S2OB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716816860;\n\tbh=2V3fts9NOsuCKxQqmyGVUvG9S79/iGl9DgDnyNLoi+c=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=sLF+S2OBdk2LCtU/xADTa1bM+4vMPE7xKNQWBxlojUao1DUlqrwxppbnWHYlsF701\n\t7DVDmoi7y5uVHOL6LNNE22aVHqlMrTa/uXQ/e/Og2VSqJv0+laEuuacfwbp21ugOsu\n\tMHEXzwUpHgbiDudc8xBsrk/7kErv4WtwZwvORwc0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240527132428.250382-1-pobrn@protonmail.com>","References":"<20240527132428.250382-1-pobrn@protonmail.com>","Subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 27 May 2024 14:34:19 +0100","Message-ID":"<171681685939.668709.5360171995976012742@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":29624,"web_url":"https://patchwork.libcamera.org/comment/29624/","msgid":"<20240527162416.GE1490@pendragon.ideasonboard.com>","date":"2024-05-27T16:24:16","subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, May 27, 2024 at 02:34:19PM +0100, Kieran Bingham wrote:\n> Quoting Barnabás Pőcze (2024-05-27 14:24:29)\n> > GCC 14 thinks `rects` is a \"possibly dangling reference to a temporary\":\n> > \n> > /libcamera/src/android/camera_capabilities.cpp: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n> > /libcamera/src/android/camera_capabilities.cpp:1084:46: error: possibly dangling reference to a temporary [-Werror=dangling-reference]\n> > 1084 | const Span& rects =\n> > | ^~~~~\n> > /libcamera/src/android/camera_capabilities.cpp:1085:83: note: the temporary was destroyed at the end of the full expression ‘(& properties)->libcamera::ControlList::get >(libcamera::properties::PixelArrayActiveAreas).std::optional >::value_or >(libcamera::Span())’\n> > 1085 | properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~\n> > \n> > The return value of `value_or()` is indeed a temporary, but\n> > binding it to a reference extends its lifetime. Avoid the\n> > warning by not using a reference; this does not make much\n> > difference since `value_or()` does not return a reference.\n> \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\nI'm working on adding gcc 14 tests in our CI.\n\n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > src/android/camera_capabilities.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 6f4d48de..71043e12 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -1081,7 +1081,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > }\n> > \n> > {\n> > - const Span &rects =\n> > + const Span rects =\n> > properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > std::vector data{\n> > static_cast(rects[0].x),","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 9A781BD87C\n\tfor ;\n\tMon, 27 May 2024 16:24:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D63B634B1;\n\tMon, 27 May 2024 18:24:31 +0200 (CEST)","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 E4091634AD\n\tfor ;\n\tMon, 27 May 2024 18:24:28 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 26405710;\n\tMon, 27 May 2024 18:24:26 +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=\"W3IQg5MD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716827066;\n\tbh=vQJ2AjBm2bkjmPJI+8rc/ELz8h7Bsjl9BE65UotFbOQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=W3IQg5MDvCC+E72Ytmp1BfWP97sjZCCKNwLVCCM8e6ZAToR+RcMTvCOLk1OMT00dl\n\tSsgrOwCR4LBg+rA360pF9qbJMj5iFXsqVR0Mp+aiUh4xzpYQ7fCKR3vLm8+0af3pCs\n\tbHD5D3EYpSfpRYgwzvm1wEMXSKTAaGeeybkNQFfc=","Date":"Mon, 27 May 2024 19:24:16 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","Message-ID":"<20240527162416.GE1490@pendragon.ideasonboard.com>","References":"<20240527132428.250382-1-pobrn@protonmail.com>\n\t<171681685939.668709.5360171995976012742@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<171681685939.668709.5360171995976012742@ping.linuxembedded.co.uk>","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":29625,"web_url":"https://patchwork.libcamera.org/comment/29625/","msgid":"<20240527170658.GA9948@pendragon.ideasonboard.com>","date":"2024-05-27T17:06:58","subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, May 27, 2024 at 07:24:16PM +0300, Laurent Pinchart wrote:\n> On Mon, May 27, 2024 at 02:34:19PM +0100, Kieran Bingham wrote:\n> > Quoting Barnabás Pőcze (2024-05-27 14:24:29)\n> > > GCC 14 thinks `rects` is a \"possibly dangling reference to a temporary\":\n> > > \n> > > /libcamera/src/android/camera_capabilities.cpp: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n> > > /libcamera/src/android/camera_capabilities.cpp:1084:46: error: possibly dangling reference to a temporary [-Werror=dangling-reference]\n> > > 1084 | const Span& rects =\n> > > | ^~~~~\n> > > /libcamera/src/android/camera_capabilities.cpp:1085:83: note: the temporary was destroyed at the end of the full expression ‘(& properties)->libcamera::ControlList::get >(libcamera::properties::PixelArrayActiveAreas).std::optional >::value_or >(libcamera::Span())’\n> > > 1085 | properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~\n> > > \n> > > The return value of `value_or()` is indeed a temporary, but\n> > > binding it to a reference extends its lifetime. Avoid the\n> > > warning by not using a reference; this does not make much\n> > > difference since `value_or()` does not return a reference.\n\nActually, I'd like to update the commit message. The issue isn't so much\nthat value_or() returns a temporary, but that the parameter passed to\nvalue_or() is destroyed at the end of the statement. Due to copy elision\n(I think), value_or() may return the instance it receives as a\nparameter.\n\nBarnabás, Would the following commit message work for you (and do you\nthink it's accurate) ?\n\n--------\nThe return value of `value_or()` is a temporary, and binding it to a\nreference extends its lifetime. However, the parameter passed to the\n`value_or()` function is destroyed at the end of the statement. With\ncopy elision, the function may effectively return the instance that it\nreceives as a parameter, leading to a dangling reference to a temporary.\n\nFix this by storing a copy of the value. As Span is a lightweight class,\nthe overhead is negligible.\n--------\n\n> > Reviewed-by: Kieran Bingham \n> \n> Reviewed-by: Laurent Pinchart \n> \n> I'm working on adding gcc 14 tests in our CI.\n> \n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > src/android/camera_capabilities.cpp | 2 +-\n> > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index 6f4d48de..71043e12 100644\n> > > --- a/src/android/camera_capabilities.cpp\n> > > +++ b/src/android/camera_capabilities.cpp\n> > > @@ -1081,7 +1081,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > > }\n> > > \n> > > {\n> > > - const Span &rects =\n> > > + const Span rects =\n> > > properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > > std::vector data{\n> > > static_cast(rects[0].x),","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 B2458BDE6B\n\tfor ;\n\tMon, 27 May 2024 17:07:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC823634B1;\n\tMon, 27 May 2024 19:07:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 51CF0634AD\n\tfor ;\n\tMon, 27 May 2024 19:07:09 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BA62B1586;\n\tMon, 27 May 2024 19:07:06 +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=\"RwXhXc5R\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716829626;\n\tbh=7JqN7NaxKQw8HjHLMDrACeHKY/aIU3E0iTxGuf2MMt4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RwXhXc5Rtmuj1OF+5OSm1OcPVmk9n/GToHbILMs2PRDLpxxssLf25vfdXW08Q+qXx\n\txwIsCqmRGSL2UwKxi41/j6uqXnBZ/3y65KaQzfBuiemKHyzSFqqUNUsPnCn6cz/7JB\n\tg0Upcvkp+y0QGjdPpaF+jI5WlOL5S9UREDa1uFis=","Date":"Mon, 27 May 2024 20:06:58 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","Message-ID":"<20240527170658.GA9948@pendragon.ideasonboard.com>","References":"<20240527132428.250382-1-pobrn@protonmail.com>\n\t<171681685939.668709.5360171995976012742@ping.linuxembedded.co.uk>\n\t<20240527162416.GE1490@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240527162416.GE1490@pendragon.ideasonboard.com>","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":29627,"web_url":"https://patchwork.libcamera.org/comment/29627/","msgid":"<2cJOON9kMAwddw5jLCI6CNj5dQjfYMBABxu5y04EJPAwWgA35uauRAcxl0_GhrfiLm78y3ryr9aemg5Nk41Nuyy2OVLS6TcxgHN7F99zKQ8=@protonmail.com>","date":"2024-05-27T18:32:04","subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. május 27., hétfő 19:06 keltezéssel, Laurent Pinchart írta:\n\n> On Mon, May 27, 2024 at 07:24:16PM +0300, Laurent Pinchart wrote:\n> > On Mon, May 27, 2024 at 02:34:19PM +0100, Kieran Bingham wrote:\n> > > Quoting Barnabás Pőcze (2024-05-27 14:24:29)\n> > > > GCC 14 thinks `rects` is a \"possibly dangling reference to a temporary\":\n> > > >\n> > > > /libcamera/src/android/camera_capabilities.cpp: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n> > > > /libcamera/src/android/camera_capabilities.cpp:1084:46: error: possibly dangling reference to a temporary [-Werror=dangling-reference]\n> > > > 1084 | const Span& rects =\n> > > > | ^~~~~\n> > > > /libcamera/src/android/camera_capabilities.cpp:1085:83: note: the temporary was destroyed at the end of the full expression ‘(& properties)->libcamera::ControlList::get >(libcamera::properties::PixelArrayActiveAreas).std::optional >::value_or >(libcamera::Span())’\n> > > > 1085 | properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~\n> > > >\n> > > > The return value of `value_or()` is indeed a temporary, but\n> > > > binding it to a reference extends its lifetime. Avoid the\n> > > > warning by not using a reference; this does not make much\n> > > > difference since `value_or()` does not return a reference.\n> \n> Actually, I'd like to update the commit message. The issue isn't so much\n> that value_or() returns a temporary, but that the parameter passed to\n> value_or() is destroyed at the end of the statement. Due to copy elision\n> (I think), value_or() may return the instance it receives as a\n> parameter.\n\nI am not entirely sure about that. Note https://en.cppreference.com/w/cpp/utility/optional/value_or\nspecifically:\n\n 2) Equivalent to bool(*this) ? std::move(**this) : static_cast(std::forward(default_value)).\n\nIn my reading when returning the default value, a new value of type T\nis constructed from the argument (which is a forwarding reference).\n\nSo in this specific case, I would say that we're constructing a `Span`\nfrom a `Span&&` (the argument), and returning that, so it\nis a new separate object from the argument.\n\nI am fairly certain GCC is in the wrong here, see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115229\nI opened that issue after running into specific warning. Note that if you remove\nthe `int *p` part from the code in the issue then the warning disappears.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Barnabás, Would the following commit message work for you (and do you\n> think it's accurate) ?\n> \n> --------\n> The return value of `value_or()` is a temporary, and binding it to a\n> reference extends its lifetime. However, the parameter passed to the\n> `value_or()` function is destroyed at the end of the statement. With\n> copy elision, the function may effectively return the instance that it\n> receives as a parameter, leading to a dangling reference to a temporary.\n> \n> Fix this by storing a copy of the value. As Span is a lightweight class,\n> the overhead is negligible.\n> --------\n> \n> > > Reviewed-by: Kieran Bingham \n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > I'm working on adding gcc 14 tests in our CI.\n> >\n> > > > Signed-off-by: Barnabás Pőcze \n> > > > ---\n> > > > src/android/camera_capabilities.cpp | 2 +-\n> > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > > index 6f4d48de..71043e12 100644\n> > > > --- a/src/android/camera_capabilities.cpp\n> > > > +++ b/src/android/camera_capabilities.cpp\n> > > > @@ -1081,7 +1081,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > > > }\n> > > >\n> > > > {\n> > > > - const Span &rects =\n> > > > + const Span rects =\n> > > > properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > > > std::vector data{\n> > > > static_cast(rects[0].x),\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 B0B52BD87C\n\tfor ;\n\tMon, 27 May 2024 18:32:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F2CCE634B1;\n\tMon, 27 May 2024 20:32:10 +0200 (CEST)","from mail-40131.protonmail.ch (mail-40131.protonmail.ch\n\t[185.70.40.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9E95634AD\n\tfor ;\n\tMon, 27 May 2024 20:32:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"RjTcd6Ga\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1716834727; x=1717093927;\n\tbh=R8tb6Lgg56r3AptqwZQt7PsWvOyZhor9WHt72V6sy9s=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=RjTcd6Gaa1COT6Bibjnc4nHrie9j+BDUzJWaIMwOlPL0cyvpzbRwiCB9RF0WAYhLA\n\t2R1cVQKLhR+fPIxMapg4hxAsNuPsGj+A+YZSLpia2/HjV5MKBwacAVwHFGuijo6Sur\n\t29UySQFQxfDtuPrIG0dXaj+/REfCfPXrYfNH+9aAkSC8BX7YwZh3p4yf/EFzDRbTRd\n\thVXKcXnjY9U5KVWIAWIWhAQvlWQshiST6fOALJlPwffFijjXfuQYGUhBgVt2HVRej/\n\tUb3Cxkf6A6B172Mxe9OtObfbB/xIqZbWYUixFQwWVqLKZqHFa8o6EKQLiZ5eqyv7X9\n\tCwSpVI19zRrKA==","Date":"Mon, 27 May 2024 18:32:04 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","Message-ID":"<2cJOON9kMAwddw5jLCI6CNj5dQjfYMBABxu5y04EJPAwWgA35uauRAcxl0_GhrfiLm78y3ryr9aemg5Nk41Nuyy2OVLS6TcxgHN7F99zKQ8=@protonmail.com>","In-Reply-To":"<20240527170658.GA9948@pendragon.ideasonboard.com>","References":"<20240527132428.250382-1-pobrn@protonmail.com>\n\t<171681685939.668709.5360171995976012742@ping.linuxembedded.co.uk>\n\t<20240527162416.GE1490@pendragon.ideasonboard.com>\n\t<20240527170658.GA9948@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"37dda5c112863117e4e50e5480d6161243ca9c52","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":29628,"web_url":"https://patchwork.libcamera.org/comment/29628/","msgid":"<20240527205339.GC12397@pendragon.ideasonboard.com>","date":"2024-05-27T20:53:39","subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nOn Mon, May 27, 2024 at 06:32:04PM +0000, Barnabás Pőcze wrote:\n> 2024. május 27., hétfő 19:06 keltezéssel, Laurent Pinchart írta:\n> > On Mon, May 27, 2024 at 07:24:16PM +0300, Laurent Pinchart wrote:\n> > > On Mon, May 27, 2024 at 02:34:19PM +0100, Kieran Bingham wrote:\n> > > > Quoting Barnabás Pőcze (2024-05-27 14:24:29)\n> > > > > GCC 14 thinks `rects` is a \"possibly dangling reference to a temporary\":\n> > > > >\n> > > > > /libcamera/src/android/camera_capabilities.cpp: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n> > > > > /libcamera/src/android/camera_capabilities.cpp:1084:46: error: possibly dangling reference to a temporary [-Werror=dangling-reference]\n> > > > > 1084 | const Span& rects =\n> > > > > | ^~~~~\n> > > > > /libcamera/src/android/camera_capabilities.cpp:1085:83: note: the temporary was destroyed at the end of the full expression ‘(& properties)->libcamera::ControlList::get >(libcamera::properties::PixelArrayActiveAreas).std::optional >::value_or >(libcamera::Span())’\n> > > > > 1085 | properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > > > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~\n> > > > >\n> > > > > The return value of `value_or()` is indeed a temporary, but\n> > > > > binding it to a reference extends its lifetime. Avoid the\n> > > > > warning by not using a reference; this does not make much\n> > > > > difference since `value_or()` does not return a reference.\n> > \n> > Actually, I'd like to update the commit message. The issue isn't so much\n> > that value_or() returns a temporary, but that the parameter passed to\n> > value_or() is destroyed at the end of the statement. Due to copy elision\n> > (I think), value_or() may return the instance it receives as a\n> > parameter.\n> \n> I am not entirely sure about that. Note https://en.cppreference.com/w/cpp/utility/optional/value_or\n> specifically:\n> \n> 2) Equivalent to bool(*this) ? std::move(**this) : static_cast(std::forward(default_value)).\n> \n> In my reading when returning the default value, a new value of type T\n> is constructed from the argument (which is a forwarding reference).\n> \n> So in this specific case, I would say that we're constructing a `Span`\n> from a `Span&&` (the argument), and returning that, so it\n> is a new separate object from the argument.\n> \n> I am fairly certain GCC is in the wrong here, see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115229\n> I opened that issue after running into specific warning. Note that if you remove\n> the `int *p` part from the code in the issue then the warning disappears.\n\nI *think* you're right, but I can't make up my mind for sure here. I'm\nnot even totally sure about which temporary gcc think the dangling\nreference relates to :-S\n\nI'll use your original commit message.\n\n> > Barnabás, Would the following commit message work for you (and do you\n> > think it's accurate) ?\n> > \n> > --------\n> > The return value of `value_or()` is a temporary, and binding it to a\n> > reference extends its lifetime. However, the parameter passed to the\n> > `value_or()` function is destroyed at the end of the statement. With\n> > copy elision, the function may effectively return the instance that it\n> > receives as a parameter, leading to a dangling reference to a temporary.\n> > \n> > Fix this by storing a copy of the value. As Span is a lightweight class,\n> > the overhead is negligible.\n> > --------\n> > \n> > > > Reviewed-by: Kieran Bingham \n> > >\n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > I'm working on adding gcc 14 tests in our CI.\n> > >\n> > > > > Signed-off-by: Barnabás Pőcze \n> > > > > ---\n> > > > > src/android/camera_capabilities.cpp | 2 +-\n> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > >\n> > > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > > > index 6f4d48de..71043e12 100644\n> > > > > --- a/src/android/camera_capabilities.cpp\n> > > > > +++ b/src/android/camera_capabilities.cpp\n> > > > > @@ -1081,7 +1081,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > > > > }\n> > > > >\n> > > > > {\n> > > > > - const Span &rects =\n> > > > > + const Span rects =\n> > > > > properties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > > > > std::vector data{\n> > > > > static_cast(rects[0].x),","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 C2CCABDE6B\n\tfor ;\n\tMon, 27 May 2024 20:53:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CAA09634B1;\n\tMon, 27 May 2024 22:53:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74724634AD\n\tfor ;\n\tMon, 27 May 2024 22:53:50 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D90ED66F;\n\tMon, 27 May 2024 22:53:47 +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=\"UZh0cpNy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716843228;\n\tbh=8mCalm5OnmoGl5A0bR/DXWrxVkqqt5silQ9m+MaiVI0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UZh0cpNybQNyZgIpmBjoQC8VoOYTTM2r2XoH7pmdRS//pDnnmOGtiF5yk4CZy+0XT\n\t/eXAV9rltg53Bfn1atGLo0h+UDY4JeP49UPcVVcgPsG37x6IaZPt0EIsNEZbtp2ejg\n\t0w6/GW9doUlYQi9jH3ea4w1JQk/Js7vn+FP2SDTw=","Date":"Mon, 27 May 2024 23:53:39 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] android: camera_capabilities: Fix GCC 14 warning","Message-ID":"<20240527205339.GC12397@pendragon.ideasonboard.com>","References":"<20240527132428.250382-1-pobrn@protonmail.com>\n\t<171681685939.668709.5360171995976012742@ping.linuxembedded.co.uk>\n\t<20240527162416.GE1490@pendragon.ideasonboard.com>\n\t<20240527170658.GA9948@pendragon.ideasonboard.com>\n\t<2cJOON9kMAwddw5jLCI6CNj5dQjfYMBABxu5y04EJPAwWgA35uauRAcxl0_GhrfiLm78y3ryr9aemg5Nk41Nuyy2OVLS6TcxgHN7F99zKQ8=@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<2cJOON9kMAwddw5jLCI6CNj5dQjfYMBABxu5y04EJPAwWgA35uauRAcxl0_GhrfiLm78y3ryr9aemg5Nk41Nuyy2OVLS6TcxgHN7F99zKQ8=@protonmail.com>","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\" "}}]