[{"id":15462,"web_url":"https://patchwork.libcamera.org/comment/15462/","msgid":"","date":"2021-03-04T00:00:35","subject":"Re: [libcamera-devel] [PATCH] uvcvideo: Use auto variable to avoid\n\trange loop warnings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Khem,\n\nThank you for the patch.\n\nOn Wed, Mar 03, 2021 at 03:21:26PM -0800, Khem Raj wrote:\n> With c++17 loop range bases are defined where copy is obvious since\n> iterator returns a copy and not reference, gcc11 will emit a warning\n> about this\n> \n> uvcvideo.cpp:432:33: error: loop variable 'name' of type 'const string&' {aka 'const std::__cxx11::basic_string r>&'} binds to a temporary constructed from type 'const char* const' [-Werror=range-loop-construct]\n> | 432 | for (const std::string &name : { \"idVendor\", \"idProduct\" }) {\n> | | ^~~~\n\nLooks like I should add gcc 11 to my build tests :-)\n\n> Therefore making it explicit is better\n> \n> Signed-off-by: Khem Raj \n> ---\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 031f96e2..ef23ece7 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -429,7 +429,7 @@ std::string PipelineHandlerUVC::generateId(const UVCCameraData *data)\n> \n> \t/* Creata a device ID from the USB devices vendor and product ID. */\n> \tstd::string deviceId;\n> -\tfor (const std::string &name : { \"idVendor\", \"idProduct\" }) {\n> +\tfor (const auto name : { \"idVendor\", \"idProduct\" }) {\n\nWhat would you think about making this\n\n\tfor (const char *name : { \"idVendor\", \"idProduct\" }) {\n\n? We tend to only use auto when the explicit type is either impossible\nto type, or would be too long, as an explicit type makes it easier for\nthe reader to know the type of the variable. I however have to say that\nusing auto here since the beginning would have prevented this bug from\nhappening in the first place. I'll let you decide what you think is\nbest, and if you opt for const char *name, I can change this when\napplying, there's no need to resend the patch.\n\nReviewed-by: Laurent Pinchart \n\n> \t\tstd::ifstream file(path + \"/../\" + name);\n> \n> \t\tif (!file.is_open())","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 2101ABD1F1\n\tfor ;\n\tThu, 4 Mar 2021 00:01:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A2186602E7;\n\tThu, 4 Mar 2021 01:01:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D67360106\n\tfor ;\n\tThu, 4 Mar 2021 01:01:06 +0100 (CET)","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 8142E31A;\n\tThu, 4 Mar 2021 01:01:05 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nfp2mNXy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614816065;\n\tbh=oDgD0whF0FoVCEp5MZ20HuBfOX4n76EwdlGZ2AyH7oM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nfp2mNXyxRqzWyzBVtopjjmCzTRkIE19OkmTszaOOT/1YnZsmRRmfbg7+IUPNodEC\n\tbfJkKU250yuTTsUyvLoN8lCX5+6OsVfA9jVpOAOjjDswMolwRdhekBsfF5t02720sp\n\tKjAxqssGVLNaCXGsNh8kmZHehJP78tSrqh+6dVE4=","Date":"Thu, 4 Mar 2021 02:00:35 +0200","From":"Laurent Pinchart ","To":"Khem Raj ","Message-ID":"","References":"<20210303232126.3369069-1-raj.khem@gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210303232126.3369069-1-raj.khem@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH] uvcvideo: Use auto variable to avoid\n\trange loop warnings","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15548,"web_url":"https://patchwork.libcamera.org/comment/15548/","msgid":"","date":"2021-03-09T00:07:06","subject":"Re: [libcamera-devel] [PATCH] uvcvideo: Use auto variable to avoid\n\trange loop warnings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Khem,\n\nOn Thu, Mar 04, 2021 at 02:00:37AM +0200, Laurent Pinchart wrote:\n> On Wed, Mar 03, 2021 at 03:21:26PM -0800, Khem Raj wrote:\n> > With c++17 loop range bases are defined where copy is obvious since\n> > iterator returns a copy and not reference, gcc11 will emit a warning\n> > about this\n> > \n> > uvcvideo.cpp:432:33: error: loop variable 'name' of type 'const string&' {aka 'const std::__cxx11::basic_string > r>&'} binds to a temporary constructed from type 'const char* const' [-Werror=range-loop-construct]\n> > | 432 | for (const std::string &name : { \"idVendor\", \"idProduct\" }) {\n> > | | ^~~~\n> \n> Looks like I should add gcc 11 to my build tests :-)\n> \n> > Therefore making it explicit is better\n> > \n> > Signed-off-by: Khem Raj \n> > ---\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 031f96e2..ef23ece7 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -429,7 +429,7 @@ std::string PipelineHandlerUVC::generateId(const UVCCameraData *data)\n> > \n> > \t/* Creata a device ID from the USB devices vendor and product ID. */\n> > \tstd::string deviceId;\n> > -\tfor (const std::string &name : { \"idVendor\", \"idProduct\" }) {\n> > +\tfor (const auto name : { \"idVendor\", \"idProduct\" }) {\n> \n> What would you think about making this\n> \n> \tfor (const char *name : { \"idVendor\", \"idProduct\" }) {\n> \n> ? We tend to only use auto when the explicit type is either impossible\n> to type, or would be too long, as an explicit type makes it easier for\n> the reader to know the type of the variable. I however have to say that\n> using auto here since the beginning would have prevented this bug from\n> happening in the first place. I'll let you decide what you think is\n> best, and if you opt for const char *name, I can change this when\n> applying, there's no need to resend the patch.\n\nWould this change be OK for you ?\n\n> Reviewed-by: Laurent Pinchart \n> \n> > \t\tstd::ifstream file(path + \"/../\" + name);\n> > \n> > \t\tif (!file.is_open())","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 EFE99BD1F1\n\tfor ;\n\tTue, 9 Mar 2021 00:07:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F1DE68A9C;\n\tTue, 9 Mar 2021 01:07:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B7E31602E4\n\tfor ;\n\tTue, 9 Mar 2021 01:07:37 +0100 (CET)","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 1406E527;\n\tTue, 9 Mar 2021 01:07:36 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lCrNfQbs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615248457;\n\tbh=YMDjCpXQBTpbftPCl+pwd58s3SaH1QPIluqRyQPQoF4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lCrNfQbsIQLI3ihk8R0TrT+gsedU5M4qc50g4aSlubnjVZwmY/3FthnsHg50yXGaq\n\t9YOqo8XGZQ9hdGy/0pOgcwOujgRVm0CAnjhv7TCp6Bu0iiG7G3Ba9C0ElBtITh2uU4\n\tW5igDvX7dUdJx2i9YCvPTjYAqoYuAnrIPPNkG3vM=","Date":"Tue, 9 Mar 2021 02:07:06 +0200","From":"Laurent Pinchart ","To":"Khem Raj ","Message-ID":"","References":"<20210303232126.3369069-1-raj.khem@gmail.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] uvcvideo: Use auto variable to avoid\n\trange loop warnings","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15549,"web_url":"https://patchwork.libcamera.org/comment/15549/","msgid":"","date":"2021-03-09T00:13:57","subject":"Re: [libcamera-devel] [PATCH] uvcvideo: Use auto variable to avoid\n\trange loop warnings","submitter":{"id":62,"url":"https://patchwork.libcamera.org/api/people/62/","name":"Khem Raj","email":"raj.khem@gmail.com"},"content":"On Mon, Mar 8, 2021 at 4:07 PM Laurent Pinchart\n wrote:\n>\n> Hi Khem,\n>\n> On Thu, Mar 04, 2021 at 02:00:37AM +0200, Laurent Pinchart wrote:\n> > On Wed, Mar 03, 2021 at 03:21:26PM -0800, Khem Raj wrote:\n> > > With c++17 loop range bases are defined where copy is obvious since\n> > > iterator returns a copy and not reference, gcc11 will emit a warning\n> > > about this\n> > >\n> > > uvcvideo.cpp:432:33: error: loop variable 'name' of type 'const string&' {aka 'const std::__cxx11::basic_string > > r>&'} binds to a temporary constructed from type 'const char* const' [-Werror=range-loop-construct]\n> > > | 432 | for (const std::string &name : { \"idVendor\", \"idProduct\" }) {\n> > > | | ^~~~\n> >\n> > Looks like I should add gcc 11 to my build tests :-)\n> >\n> > > Therefore making it explicit is better\n> > >\n> > > Signed-off-by: Khem Raj \n> > > ---\n> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-\n> > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index 031f96e2..ef23ece7 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -429,7 +429,7 @@ std::string PipelineHandlerUVC::generateId(const UVCCameraData *data)\n> > >\n> > > /* Creata a device ID from the USB devices vendor and product ID. */\n> > > std::string deviceId;\n> > > - for (const std::string &name : { \"idVendor\", \"idProduct\" }) {\n> > > + for (const auto name : { \"idVendor\", \"idProduct\" }) {\n> >\n> > What would you think about making this\n> >\n> > for (const char *name : { \"idVendor\", \"idProduct\" }) {\n> >\n> > ? We tend to only use auto when the explicit type is either impossible\n> > to type, or would be too long, as an explicit type makes it easier for\n> > the reader to know the type of the variable. I however have to say that\n> > using auto here since the beginning would have prevented this bug from\n> > happening in the first place. I'll let you decide what you think is\n> > best, and if you opt for const char *name, I can change this when\n> > applying, there's no need to resend the patch.\n>\n> Would this change be OK for you ?\n>\n\nI have to test this out and haven't yet got to it. but I think this\nchange will be ok as well.\n\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > std::ifstream file(path + \"/../\" + name);\n> > >\n> > > if (!file.is_open())\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 E190EBD80C\n\tfor ;\n\tTue, 9 Mar 2021 00:14:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B67B6051F;\n\tTue, 9 Mar 2021 01:14:27 +0100 (CET)","from mail-qv1-xf2e.google.com (mail-qv1-xf2e.google.com\n\t[IPv6:2607:f8b0:4864:20::f2e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57E0B602E4\n\tfor ;\n\tTue, 9 Mar 2021 01:14:25 +0100 (CET)","by mail-qv1-xf2e.google.com with SMTP id t1so5573842qvj.8\n\tfor ;\n\tMon, 08 Mar 2021 16:14:25 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"LX15eAZB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=NRZAUcCSmWf74RVwMbKYp5sHTLxNxkqCIi2roNuo9hA=;\n\tb=LX15eAZBb3zRqYYD5ULhd4JFQ0brTFM7XoiGEJHnRVFdC2kc6VkiRzjqxAfaM2BRJV\n\tESvDh7sUk6p2f/gUBNiHSjFevn2Kv2dq1L6vj5wgXTkNfNJce5h7ryffIBq7Zhz4YLNg\n\t3J2KpEUpSpaALexVcysHP0hB2RX3H1QiKp6+jzfO47NKdmWkxLu3Ux9P+mGj/g71htjy\n\tCs95dlYXeQGgoKP62fn9t4QiBuDKaM7+R50PUihpgiGyyCUKQtdvdYcYoaz50WTMAY0Q\n\thnedmNQBLDYJlD7NAjXo3jC5oeCeDiKLU3vdiscPAg/w9ucmOeA0SVvPnNiojj5DUTLo\n\t/2nQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=NRZAUcCSmWf74RVwMbKYp5sHTLxNxkqCIi2roNuo9hA=;\n\tb=uHOCDeegyHePppRxOYvrdzXue5s8FtlZpBbdtdLaROH/CqR9D307y60xNLeesggqk6\n\tR6IGPkS7BvCmRVwuwvBbtgZsrFkk9/dycpA1mwydDrz2DU41nJixdQ5UykBSed6s/x91\n\tNZsT9iIKKj2AYpQyS3Ygvqk2Zdxg38Q/EKUL9ARGu9HlZklF4Ioy6HitD1iSy4/IhsKX\n\tPEww10NWxihxBnRDSo3LA4c88rEoST3KHj6BU5ehrsTQubf1T/BgRGoeAnuDrFFVR6Xm\n\t0MVvMPkWN62XLLE1szV1wpZTjuIVKUzvoxw2HwF6YLOecZ0Lphm5e3RXFsu5RYvUf1US\n\t7Ytg==","X-Gm-Message-State":"AOAM532+VJmiAYLmiNh0gOQIwKn9T58Fm6qhl37Cc1/BWGNN7DGZiy9v\n\tis5JHEypTGm9K6DjrTQijYRUGNehVNHG6Uk4SN4=","X-Google-Smtp-Source":"ABdhPJxgy1tpygLAF1RsnN3HDZzHxxOM7/z0I2OPtY1LIefNmOzpHFO2IXP9zuCBJgWyiElpTYpiF2OSByFRMPs+kK0=","X-Received":"by 2002:a0c:fe0c:: with SMTP id\n\tx12mr22630159qvr.16.1615248863896; \n\tMon, 08 Mar 2021 16:14:23 -0800 (PST)","MIME-Version":"1.0","References":"<20210303232126.3369069-1-raj.khem@gmail.com>\n\t\n\t","In-Reply-To":"","From":"Khem Raj ","Date":"Mon, 8 Mar 2021 16:13:57 -0800","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH] uvcvideo: Use auto variable to avoid\n\trange loop warnings","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]