[{"id":16953,"web_url":"https://patchwork.libcamera.org/comment/16953/","msgid":"<YJ+Ry/TMoAcdvK6f@oden.dyn.berto.se>","date":"2021-05-15T09:18:03","subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2021-05-15 07:05:09 +0300, Laurent Pinchart wrote:\n> Use the newly introduced utils::enumerate() to replace manual loop\n> counters. A local variable is needed, as utils::enumerate() requires an\n> lvalue reference.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n> Changes since v1:\n> \n> - Use structured bindings\n> ---\n>  src/v4l2/v4l2_compat_manager.cpp | 11 +++++------\n>  1 file changed, 5 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> index 90c0f0121a32..96dbcdf28f04 100644\n> --- a/src/v4l2/v4l2_compat_manager.cpp\n> +++ b/src/v4l2/v4l2_compat_manager.cpp\n> @@ -23,6 +23,7 @@\n>  #include <libcamera/camera_manager.h>\n>  \n>  #include \"libcamera/internal/log.h\"\n> +#include \"libcamera/internal/utils.h\"\n>  \n>  #include \"v4l2_camera_file.h\"\n>  \n> @@ -81,11 +82,10 @@ int V4L2CompatManager::start()\n>  \t * For each Camera registered in the system, a V4L2CameraProxy gets\n>  \t * created here to wrap a camera device.\n>  \t */\n> -\tunsigned int index = 0;\n> -\tfor (auto &camera : cm_->cameras()) {\n> +\tauto cameras = cm_->cameras();\n> +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n>  \t\tV4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n>  \t\tproxies_.emplace_back(proxy);\n> -\t\t++index;\n>  \t}\n>  \n>  \treturn 0;\n> @@ -117,11 +117,10 @@ int V4L2CompatManager::getCameraIndex(int fd)\n>  \tif (!target)\n>  \t\treturn -1;\n>  \n> -\tunsigned int index = 0;\n> -\tfor (auto &camera : cm_->cameras()) {\n> +\tauto cameras = cm_->cameras();\n> +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n>  \t\tif (camera == target)\n>  \t\t\treturn index;\n> -\t\t++index;\n>  \t}\n>  \n>  \treturn -1;\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 03944C31FA\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 15 May 2021 09:18:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 121496891A;\n\tSat, 15 May 2021 11:18:07 +0200 (CEST)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57E6F68915\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 15 May 2021 11:18:06 +0200 (CEST)","by mail-lj1-x235.google.com with SMTP id w15so1290898ljo.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 15 May 2021 02:18:06 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ta7sm1328954lff.39.2021.05.15.02.18.04\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 15 May 2021 02:18:04 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"gUE78dYm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=219rui40KGwwbB2jEnDNmgIborJzZtOw8PVHGH2Cg8M=;\n\tb=gUE78dYm7siIQm8fKiH2pQuUMAolcVfJuyYJuEDNbXHjlJ2H1eGGRgjnrmNKC+pik4\n\t0u3yq/F0RRiWGRByngEKJEjZ/uVCgeCb4Cb3FUu/LsyeGOH99CtegJagqjCEsjvjafJo\n\tW4MWdF92rQegiwTRBA+RQd8Y7WBUtx7rW5RRKqiTAPciESZsIOqeVkK7LvEhvH6uzhmF\n\t5p9QavbWnu8xN0v46t/0NZfxmNH9lxFS8BaohiOhkR0W4nHNzQCBzHDNpu24S0SePXaM\n\tw9se9H1buN14jJQvhD0P/VWN+i2zRKue/WKOWny7dKMkeRGWpnJWUEbM8+EPzdJA+Qjk\n\tuqXQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=219rui40KGwwbB2jEnDNmgIborJzZtOw8PVHGH2Cg8M=;\n\tb=MwuP8k9nWwC8b2+K2rAlQBarnDGvMshpq/YhPyPJWS8XWP/PeJvd11ZiZLM/RekNDu\n\tulxWQfnBvRvmaWeuK/6HF4eM1OClBHGDZ9a4j1tUu4+oewNTI6OeD3HUDGud5eddsxHM\n\towO97mrHo4LpXg4Eqrwc1fMOZUSsAVlYm5LvyTWbpNhsX3KUpzJr+kT8H+ggcU0+BUkY\n\tiQq3wKPnRUW0O3xNn4fCj7LS+b4N77QCAxmqvVoYadwSDcPdBNkpJq3sUbmCjwyiNfKN\n\tXU9LEYves5Aeo51w1rp3o80t/Ux0vCGpJmbsz1hU7Bv2Bbl9O36BBwa2zxN0x3bOGZIf\n\tBOaQ==","X-Gm-Message-State":"AOAM532d3/18Hzv7JQqE7aWP+PMtzQ6T58/0RL6zjUv4/s3GWNHg74pm\n\tp1LNR+M4hwyPrpHP8F0tcyjYTsCQ9STi9w==","X-Google-Smtp-Source":"ABdhPJy47o3y5iRA2t4/rZ82SqjDiPu0TKJ66atpQ2gldGsy3/I6W7HSODvDNByKMRqZ2UD5uDxtig==","X-Received":"by 2002:a05:651c:22b:: with SMTP id\n\tz11mr39724311ljn.182.1621070285478; \n\tSat, 15 May 2021 02:18:05 -0700 (PDT)","Date":"Sat, 15 May 2021 11:18:03 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<YJ+Ry/TMoAcdvK6f@oden.dyn.berto.se>","References":"<20210515040511.23294-1-laurent.pinchart@ideasonboard.com>\n\t<20210515040511.23294-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210515040511.23294-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16993,"web_url":"https://patchwork.libcamera.org/comment/16993/","msgid":"<bbeebd8d-afe7-cd5a-c059-019cad2a9e69@ideasonboard.com>","date":"2021-05-17T12:54:08","subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 15/05/2021 05:05, Laurent Pinchart wrote:\n> Use the newly introduced utils::enumerate() to replace manual loop\n> counters. A local variable is needed, as utils::enumerate() requires an\n> lvalue reference.\n\nSo this is not possible?\n\n\tfor (auto [index, camera] : utils::enumerate(cm_->cameras()) {\n\nI guess that's fine, but will it be obvious to someone trying to use it\nthat they need to move the thing to enumerate to a local variable if\nit's not there?\n\nI probably really don't want to know - but why does it need this?\nIt seems like an odd limitation, but if it's just a small corner case,\nthen I don't mind having it split out. It helps keep line lengths\nshorter anyway...?\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n> Changes since v1:\n> \n> - Use structured bindings\n> ---\n>  src/v4l2/v4l2_compat_manager.cpp | 11 +++++------\n>  1 file changed, 5 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> index 90c0f0121a32..96dbcdf28f04 100644\n> --- a/src/v4l2/v4l2_compat_manager.cpp\n> +++ b/src/v4l2/v4l2_compat_manager.cpp\n> @@ -23,6 +23,7 @@\n>  #include <libcamera/camera_manager.h>\n>  \n>  #include \"libcamera/internal/log.h\"\n> +#include \"libcamera/internal/utils.h\"\n>  \n>  #include \"v4l2_camera_file.h\"\n>  \n> @@ -81,11 +82,10 @@ int V4L2CompatManager::start()\n>  \t * For each Camera registered in the system, a V4L2CameraProxy gets\n>  \t * created here to wrap a camera device.\n>  \t */\n> -\tunsigned int index = 0;\n> -\tfor (auto &camera : cm_->cameras()) {\n> +\tauto cameras = cm_->cameras();\n> +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n>  \t\tV4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n>  \t\tproxies_.emplace_back(proxy);\n> -\t\t++index;\n>  \t}\n>  \n>  \treturn 0;\n> @@ -117,11 +117,10 @@ int V4L2CompatManager::getCameraIndex(int fd)\n>  \tif (!target)\n>  \t\treturn -1;\n>  \n> -\tunsigned int index = 0;\n> -\tfor (auto &camera : cm_->cameras()) {\n> +\tauto cameras = cm_->cameras();\n> +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n>  \t\tif (camera == target)\n>  \t\t\treturn index;\n> -\t\t++index;\n>  \t}\n>  \n>  \treturn -1;\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DA73DC31FC\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 May 2021 12:54:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FBCC68918;\n\tMon, 17 May 2021 14:54:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B718602C6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 May 2021 14:54:12 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AABAB88F;\n\tMon, 17 May 2021 14:54:11 +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=\"H0MY6H84\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621256051;\n\tbh=qN2dYyH8MXwIjrfjRF6ITlIvS4ef6VECCk7ixrvUfVU=;\n\th=Reply-To:To:References:From:Subject:Date:In-Reply-To:From;\n\tb=H0MY6H84vytrhmTL80gve+EKC71JSAzuyF3XXT55mPZkND1r4reyk/gxRMpezq9D1\n\tFBdiTMA6O78ty1qxyAg79l3y09Oq/tN0NJbms1lJ5gmNsuWQitYeedk2raFjsokvEZ\n\tHllzjLu5LQOUO6ykos+hXixlDkUH+IwuN26OpvDM=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210515040511.23294-1-laurent.pinchart@ideasonboard.com>\n\t<20210515040511.23294-3-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Organization":"Ideas on Board","Message-ID":"<bbeebd8d-afe7-cd5a-c059-019cad2a9e69@ideasonboard.com>","Date":"Mon, 17 May 2021 13:54:08 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210515040511.23294-3-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16995,"web_url":"https://patchwork.libcamera.org/comment/16995/","msgid":"<YKKbmtqzJa+hXINV@pendragon.ideasonboard.com>","date":"2021-05-17T16:36:42","subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, May 17, 2021 at 01:54:08PM +0100, Kieran Bingham wrote:\n> On 15/05/2021 05:05, Laurent Pinchart wrote:\n> > Use the newly introduced utils::enumerate() to replace manual loop\n> > counters. A local variable is needed, as utils::enumerate() requires an\n> > lvalue reference.\n> \n> So this is not possible?\n> \n> \tfor (auto [index, camera] : utils::enumerate(cm_->cameras()) {\n\nUnfortunately not :-(\n\n> I guess that's fine, but will it be obvious to someone trying to use it\n> that they need to move the thing to enumerate to a local variable if\n> it's not there?\n\nThe compiler should make it obvious:\n\n../../src/v4l2/v4l2_compat_manager.cpp: In member function ‘int V4L2CompatManager::start()’:\n../../src/v4l2/v4l2_compat_manager.cpp:85:53: error: no matching function for call to ‘enumerate(std::vector<std::shared_ptr<libcamera::Camera> >)’\n   85 |         for (auto [index, camera] : utils::enumerate(cm_->cameras())) {\n\nutils::enumerate() takes an lvalue reference, and cm_->cameras() is an\nrvalue.\n\n> I probably really don't want to know - but why does it need this?\n\nhttps://en.cppreference.com/w/cpp/language/range-for#Temporary_range_expression\n\ncm_->cameras() returns a temporay value, which is passed to\nutils::enumerate(). utils::enumerate() returns another temporary that\nstores a reference to cm_->cameras(). The lifetime of the second\ntemporary is extended to the whole loop, but the lifetime of the first\ntemporary isn't.\n\n> It seems like an odd limitation, but if it's just a small corner case,\n> then I don't mind having it split out. It helps keep line lengths\n> shorter anyway...?\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> > Changes since v1:\n> > \n> > - Use structured bindings\n> > ---\n> >  src/v4l2/v4l2_compat_manager.cpp | 11 +++++------\n> >  1 file changed, 5 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > index 90c0f0121a32..96dbcdf28f04 100644\n> > --- a/src/v4l2/v4l2_compat_manager.cpp\n> > +++ b/src/v4l2/v4l2_compat_manager.cpp\n> > @@ -23,6 +23,7 @@\n> >  #include <libcamera/camera_manager.h>\n> >  \n> >  #include \"libcamera/internal/log.h\"\n> > +#include \"libcamera/internal/utils.h\"\n> >  \n> >  #include \"v4l2_camera_file.h\"\n> >  \n> > @@ -81,11 +82,10 @@ int V4L2CompatManager::start()\n> >  \t * For each Camera registered in the system, a V4L2CameraProxy gets\n> >  \t * created here to wrap a camera device.\n> >  \t */\n> > -\tunsigned int index = 0;\n> > -\tfor (auto &camera : cm_->cameras()) {\n> > +\tauto cameras = cm_->cameras();\n> > +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n> >  \t\tV4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n> >  \t\tproxies_.emplace_back(proxy);\n> > -\t\t++index;\n> >  \t}\n> >  \n> >  \treturn 0;\n> > @@ -117,11 +117,10 @@ int V4L2CompatManager::getCameraIndex(int fd)\n> >  \tif (!target)\n> >  \t\treturn -1;\n> >  \n> > -\tunsigned int index = 0;\n> > -\tfor (auto &camera : cm_->cameras()) {\n> > +\tauto cameras = cm_->cameras();\n> > +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n> >  \t\tif (camera == target)\n> >  \t\t\treturn index;\n> > -\t\t++index;\n> >  \t}\n> >  \n> >  \treturn -1;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6DEF5C31FC\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 May 2021 16:36:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CBAE768911;\n\tMon, 17 May 2021 18:36:55 +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 BC81E602C6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 May 2021 18:36:53 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1CCE6102;\n\tMon, 17 May 2021 18:36:53 +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=\"H4xi7Nh3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621269413;\n\tbh=3Z+vYdY8ux/rKXIK5OCRby29GadecAtcWUyH+B5N1pU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H4xi7Nh3oZBTa/Tllt/wOAuk6alz2ftez6X8O+qpAtAoplq7uS/FGSeevfFJBTk7D\n\tbfowopR8tC2TneHUTXYOM/P489j0Ap20SE+8zeVA+JbEPKm99HdEuUqUroDDuShTqC\n\tnuL6ktWtaZJ/gM+7nucylrrolZmMLM3oFUbSVjCA=","Date":"Mon, 17 May 2021 19:36:42 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YKKbmtqzJa+hXINV@pendragon.ideasonboard.com>","References":"<20210515040511.23294-1-laurent.pinchart@ideasonboard.com>\n\t<20210515040511.23294-3-laurent.pinchart@ideasonboard.com>\n\t<bbeebd8d-afe7-cd5a-c059-019cad2a9e69@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<bbeebd8d-afe7-cd5a-c059-019cad2a9e69@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17002,"web_url":"https://patchwork.libcamera.org/comment/17002/","msgid":"<YKOaCHtH+K70PBJj@pendragon.ideasonboard.com>","date":"2021-05-18T10:42:16","subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, May 17, 2021 at 07:36:43PM +0300, Laurent Pinchart wrote:\n> On Mon, May 17, 2021 at 01:54:08PM +0100, Kieran Bingham wrote:\n> > On 15/05/2021 05:05, Laurent Pinchart wrote:\n> > > Use the newly introduced utils::enumerate() to replace manual loop\n> > > counters. A local variable is needed, as utils::enumerate() requires an\n> > > lvalue reference.\n> > \n> > So this is not possible?\n> > \n> > \tfor (auto [index, camera] : utils::enumerate(cm_->cameras()) {\n> \n> Unfortunately not :-(\n> \n> > I guess that's fine, but will it be obvious to someone trying to use it\n> > that they need to move the thing to enumerate to a local variable if\n> > it's not there?\n> \n> The compiler should make it obvious:\n> \n> ../../src/v4l2/v4l2_compat_manager.cpp: In member function ‘int V4L2CompatManager::start()’:\n> ../../src/v4l2/v4l2_compat_manager.cpp:85:53: error: no matching function for call to ‘enumerate(std::vector<std::shared_ptr<libcamera::Camera> >)’\n>    85 |         for (auto [index, camera] : utils::enumerate(cm_->cameras())) {\n> \n> utils::enumerate() takes an lvalue reference, and cm_->cameras() is an\n> rvalue.\n\nI'll add the following to the documentation of the function:\n\n * Note that the argument to enumerate() has to be an lvalue, as the lifetime\n * of any rvalue would not be extended to the whole for loop. The compiler will\n * complain if an rvalue is passed to the function, in which case it should be\n * stored in a local variable before the loop.\n\n> > I probably really don't want to know - but why does it need this?\n> \n> https://en.cppreference.com/w/cpp/language/range-for#Temporary_range_expression\n> \n> cm_->cameras() returns a temporay value, which is passed to\n> utils::enumerate(). utils::enumerate() returns another temporary that\n> stores a reference to cm_->cameras(). The lifetime of the second\n> temporary is extended to the whole loop, but the lifetime of the first\n> temporary isn't.\n> \n> > It seems like an odd limitation, but if it's just a small corner case,\n> > then I don't mind having it split out. It helps keep line lengths\n> > shorter anyway...?\n> > \n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > > Changes since v1:\n> > > \n> > > - Use structured bindings\n> > > ---\n> > >  src/v4l2/v4l2_compat_manager.cpp | 11 +++++------\n> > >  1 file changed, 5 insertions(+), 6 deletions(-)\n> > > \n> > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > > index 90c0f0121a32..96dbcdf28f04 100644\n> > > --- a/src/v4l2/v4l2_compat_manager.cpp\n> > > +++ b/src/v4l2/v4l2_compat_manager.cpp\n> > > @@ -23,6 +23,7 @@\n> > >  #include <libcamera/camera_manager.h>\n> > >  \n> > >  #include \"libcamera/internal/log.h\"\n> > > +#include \"libcamera/internal/utils.h\"\n> > >  \n> > >  #include \"v4l2_camera_file.h\"\n> > >  \n> > > @@ -81,11 +82,10 @@ int V4L2CompatManager::start()\n> > >  \t * For each Camera registered in the system, a V4L2CameraProxy gets\n> > >  \t * created here to wrap a camera device.\n> > >  \t */\n> > > -\tunsigned int index = 0;\n> > > -\tfor (auto &camera : cm_->cameras()) {\n> > > +\tauto cameras = cm_->cameras();\n> > > +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n> > >  \t\tV4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n> > >  \t\tproxies_.emplace_back(proxy);\n> > > -\t\t++index;\n> > >  \t}\n> > >  \n> > >  \treturn 0;\n> > > @@ -117,11 +117,10 @@ int V4L2CompatManager::getCameraIndex(int fd)\n> > >  \tif (!target)\n> > >  \t\treturn -1;\n> > >  \n> > > -\tunsigned int index = 0;\n> > > -\tfor (auto &camera : cm_->cameras()) {\n> > > +\tauto cameras = cm_->cameras();\n> > > +\tfor (auto [index, camera] : utils::enumerate(cameras)) {\n> > >  \t\tif (camera == target)\n> > >  \t\t\treturn index;\n> > > -\t\t++index;\n> > >  \t}\n> > >  \n> > >  \treturn -1;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1D2F2C31FF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 May 2021 10:42:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DB6F6891E;\n\tTue, 18 May 2021 12:42:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 711C9602B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 May 2021 12:42:17 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D7584102;\n\tTue, 18 May 2021 12:42:16 +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=\"bC9Qifuy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621334537;\n\tbh=dx5isoKOVdIDuL8MReCnlMLalO93QRqPrfps5QaRFcE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bC9QifuyFYSOLZJFhT8QyiLcG3VpV1Kz0BXsXe1qXjwmeBVJVwlrqyuDSCwqjMXUW\n\tejDOpCZ/FD7XnjTPdYkvJzdlBU4TD6MRAtGOlvdi593eR/dDjrCNV0m3NdyxcyFcjx\n\tWliP7TJBmg1YXXw26EoV4ou9Q4onWyMAG4+s1iDg=","Date":"Tue, 18 May 2021 13:42:16 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YKOaCHtH+K70PBJj@pendragon.ideasonboard.com>","References":"<20210515040511.23294-1-laurent.pinchart@ideasonboard.com>\n\t<20210515040511.23294-3-laurent.pinchart@ideasonboard.com>\n\t<bbeebd8d-afe7-cd5a-c059-019cad2a9e69@ideasonboard.com>\n\t<YKKbmtqzJa+hXINV@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<YKKbmtqzJa+hXINV@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17003,"web_url":"https://patchwork.libcamera.org/comment/17003/","msgid":"<31cf57ef-72d4-b790-3348-093052f58520@ideasonboard.com>","date":"2021-05-18T10:43:28","subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 18/05/2021 11:42, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Mon, May 17, 2021 at 07:36:43PM +0300, Laurent Pinchart wrote:\n>> On Mon, May 17, 2021 at 01:54:08PM +0100, Kieran Bingham wrote:\n>>> On 15/05/2021 05:05, Laurent Pinchart wrote:\n>>>> Use the newly introduced utils::enumerate() to replace manual loop\n>>>> counters. A local variable is needed, as utils::enumerate() requires an\n>>>> lvalue reference.\n>>>\n>>> So this is not possible?\n>>>\n>>> \tfor (auto [index, camera] : utils::enumerate(cm_->cameras()) {\n>>\n>> Unfortunately not :-(\n>>\n>>> I guess that's fine, but will it be obvious to someone trying to use it\n>>> that they need to move the thing to enumerate to a local variable if\n>>> it's not there?\n>>\n>> The compiler should make it obvious:\n>>\n>> ../../src/v4l2/v4l2_compat_manager.cpp: In member function ‘int V4L2CompatManager::start()’:\n>> ../../src/v4l2/v4l2_compat_manager.cpp:85:53: error: no matching function for call to ‘enumerate(std::vector<std::shared_ptr<libcamera::Camera> >)’\n>>    85 |         for (auto [index, camera] : utils::enumerate(cm_->cameras())) {\n>>\n>> utils::enumerate() takes an lvalue reference, and cm_->cameras() is an\n>> rvalue.\n> \n> I'll add the following to the documentation of the function:\n> \n>  * Note that the argument to enumerate() has to be an lvalue, as the lifetime\n>  * of any rvalue would not be extended to the whole for loop. The compiler will\n>  * complain if an rvalue is passed to the function, in which case it should be\n>  * stored in a local variable before the loop.\n\nPerfect.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 63BA2C31FF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 May 2021 10:43:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 185546891F;\n\tTue, 18 May 2021 12:43:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 391A968919\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 May 2021 12:43:31 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D7C195A9;\n\tTue, 18 May 2021 12:43:30 +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=\"eChiCWZT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621334611;\n\tbh=3nFzVUdjnGcdpDmMLxuOWGr6Hnr2rcZl/4gq+Lqal3Q=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=eChiCWZTa7J4NPshOELDkYm223aBIyalLRfT7DBo7Rjg73yhobbTFVLwTPx7KIo7/\n\tLuh1A/oMquMjuruC+yT/gAFI7iBfu7e2gS9RRAyspsqLLmFAfw0glH7aEy3n8viQ3x\n\tXsV53FXS7mxg1VQPozaqeo0U+U7KaP02vJ0HoqVo=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20210515040511.23294-1-laurent.pinchart@ideasonboard.com>\n\t<20210515040511.23294-3-laurent.pinchart@ideasonboard.com>\n\t<bbeebd8d-afe7-cd5a-c059-019cad2a9e69@ideasonboard.com>\n\t<YKKbmtqzJa+hXINV@pendragon.ideasonboard.com>\n\t<YKOaCHtH+K70PBJj@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Organization":"Ideas on Board","Message-ID":"<31cf57ef-72d4-b790-3348-093052f58520@ideasonboard.com>","Date":"Tue, 18 May 2021 11:43:28 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<YKOaCHtH+K70PBJj@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] v4l2: Replace manual loop\n\tcounters with utils::enumerate()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]