[{"id":2021,"web_url":"https://patchwork.libcamera.org/comment/2021/","msgid":"<20190625133259.GE5002@pendragon.ideasonboard.com>","date":"2019-06-25T13:32:59","subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nI would write the subject as \"Fix compilation warning\" or something\nalong those lines.\n\nOn Tue, Jun 25, 2019 at 02:15:08PM +0100, Kieran Bingham wrote:\n> GCCv6 and GCCv7 take objections to declaring a struct type when\n> using a range based iterator. This issue does not appear in GCCv8\n> \n>     event_dispatcher_poll.cpp:231:13: error: types may not be defined\n> \tin a for-range-declaration [-Werror]\n> \n> \t\tfor (const struct pollfd &pfd : pollfds) {\n> \t\t           ^~~~~~\n> \n> \tcc1plus: all warnings being treated as errors\n> \n> Removing the keyword 'struct' ensures that the compiler does not try to\n> declare the type, and instead uses the type as already defined by the\n> relevant poll.h header.\n\nI can't reproduce this with gcc 6.4.0 or 7.4.0, I wonder if it could be\nrelated to the C library. You may want to mention the exact compiler\nversions that resulted in the error.\n\nIn any case,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> Reported-by: [autobuild.buildroot.net] Thomas Petazzoni <thomas.petazzoni@bootlin.com>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/libcamera/event_dispatcher_poll.cpp | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> index 0ff99fce47ab..df9dffb2326c 100644\n> --- a/src/libcamera/event_dispatcher_poll.cpp\n> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> @@ -241,7 +241,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol\n>  \t\t{ EventNotifier::Exception, POLLPRI },\n>  \t};\n>  \n> -\tfor (const struct pollfd &pfd : pollfds) {\n> +\tfor (const pollfd &pfd : pollfds) {\n>  \t\tauto iter = notifiers_.find(pfd.fd);\n>  \t\tASSERT(iter != notifiers_.end());\n>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 7D85D61583\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Jun 2019 15:35:26 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E0B3D510;\n\tTue, 25 Jun 2019 15:35:25 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561469726;\n\tbh=LyXjcAUvRHIeUg4/nKvzaTQ06YrHXcuJfzkokutKG34=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UGH67ZeihO0nbfdabqIzDArpNXmRy7DhfXhBkkoMRpFRi0xj1YJNNfQzdoKkzw2LC\n\tJTn01/tDG9IsyuRXZtq2xtFLXGaC5l8unFu2T351veveFwk8JTt+RIV3g8AqW6LfQ0\n\tWBDVt/yyMzKF+r9jB7avlsifIwOIr7bwKfoGpJlY=","Date":"Tue, 25 Jun 2019 16:32:59 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>,\n\t\"[autobuild.buildroot.net] Thomas Petazzoni\"\n\t<thomas.petazzoni@bootlin.com>","Message-ID":"<20190625133259.GE5002@pendragon.ideasonboard.com>","References":"<20190625131508.31263-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190625131508.31263-1-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 25 Jun 2019 13:35:26 -0000"}},{"id":2022,"web_url":"https://patchwork.libcamera.org/comment/2022/","msgid":"<ef5ad6d4-848b-6631-473c-cd0f79e454e2@ideasonboard.com>","date":"2019-06-25T17:13:10","subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 25/06/2019 14:32, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> I would write the subject as \"Fix compilation warning\" or something\n> along those lines.\n\nI think the subject should state *what* it does, rather than /why/.\n\nTo fix a compilation warning (or error as we use -Werror) is the reason\nwhy we fix it :)\n\nHow about:\n\nlibcamera: event_dispatcher_poll: Remove struct keyword from for-range\n\nI'd like to make that 'remove redundant struct keyword' but I've run out\nof title space :D\n\n\n> On Tue, Jun 25, 2019 at 02:15:08PM +0100, Kieran Bingham wrote:\n>> GCCv6 and GCCv7 take objections to declaring a struct type when\n>> using a range based iterator. This issue does not appear in GCCv8\n>>\n>>     event_dispatcher_poll.cpp:231:13: error: types may not be defined\n>> \tin a for-range-declaration [-Werror]\n>>\n>> \t\tfor (const struct pollfd &pfd : pollfds) {\n>> \t\t           ^~~~~~\n>>\n>> \tcc1plus: all warnings being treated as errors\n>>\n>> Removing the keyword 'struct' ensures that the compiler does not try to\n>> declare the type, and instead uses the type as already defined by the\n>> relevant poll.h header.\n> \n> I can't reproduce this with gcc 6.4.0 or 7.4.0, I wonder if it could be\n> related to the C library. You may want to mention the exact compiler\n> versions that resulted in the error.\n\nUsing the following code sample, I have tested a range of compilers\nusing godbolt.org:\n\n#include <vector>\n#include <poll.h>\n\nvoid processNotifiers(const std::vector<struct pollfd> &pollfds) {\n    \tfor (const struct pollfd &pfd : pollfds) {\n\n        }\n}\n\nSo actually I have misinterpreted the issue. I thought it was a newly\nintroduced compiler warning. In fact, old compilers fail - while newer\ncompilers accept our current syntax.\n\n\nGodbolt shows that this failure occurs at the following versions:\n\n v4.9.3 fails\n v4.9.4 fails\n\n v5.1 fails\n v5.2 fails\n v5.3 fails\n v5.4 fails\n v5.5 fails\n\n v6.2 fails\n v6.3 fails\n\n v6.4 supported\n (and so are the versions following)\n\nThe code sample can be explored at this godbolt short-link:\n   https://godbolt.org/z/mV6ita\n\n\n\n\n> \n> In any case,\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThanks, I've updated the commit message a fair bit so I'll send a v2 anyway.\n--\nKieran\n\n\n> \n>> Reported-by: [autobuild.buildroot.net] Thomas Petazzoni <thomas.petazzoni@bootlin.com>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>  src/libcamera/event_dispatcher_poll.cpp | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n>> index 0ff99fce47ab..df9dffb2326c 100644\n>> --- a/src/libcamera/event_dispatcher_poll.cpp\n>> +++ b/src/libcamera/event_dispatcher_poll.cpp\n>> @@ -241,7 +241,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol\n>>  \t\t{ EventNotifier::Exception, POLLPRI },\n>>  \t};\n>>  \n>> -\tfor (const struct pollfd &pfd : pollfds) {\n>> +\tfor (const pollfd &pfd : pollfds) {\n>>  \t\tauto iter = notifiers_.find(pfd.fd);\n>>  \t\tASSERT(iter != notifiers_.end());\n>>  \n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["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 A329460103\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Jun 2019 19:13:15 +0200 (CEST)","from [192.168.43.26] (unknown [80.2.20.132])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A9DF3510;\n\tTue, 25 Jun 2019 19:13:14 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561482795;\n\tbh=lPqp1PEuHha8rfwwisL20EWp1Lw5UVjZgyvwLJ80p8E=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=REzdukZV+Ae3vV8/sJaRQpHiriDO2Z5Nn6CMjfPGja8MFkTuDU0aRV4T9qjSCXnDQ\n\tLLQae50kzCD30XhYjLLhhUwqdJwRDMUbW6c3G0cdcp7F3bevSBttf6ybQATiBdJrag\n\tY+ZXHKY6wLh04UVR0xx66LDmQ9adtBA6/EqesOe0=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>,\n\t\"[autobuild.buildroot.net] Thomas Petazzoni\"\n\t<thomas.petazzoni@bootlin.com>","References":"<20190625131508.31263-1-kieran.bingham@ideasonboard.com>\n\t<20190625133259.GE5002@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<ef5ad6d4-848b-6631-473c-cd0f79e454e2@ideasonboard.com>","Date":"Tue, 25 Jun 2019 18:13:10 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190625133259.GE5002@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 25 Jun 2019 17:13:15 -0000"}},{"id":2023,"web_url":"https://patchwork.libcamera.org/comment/2023/","msgid":"<20190625172907.GH5002@pendragon.ideasonboard.com>","date":"2019-06-25T17:29:07","subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Jun 25, 2019 at 06:13:10PM +0100, Kieran Bingham wrote:\n> On 25/06/2019 14:32, Laurent Pinchart wrote:\n> > Hi Kieran,\n> > \n> > Thank you for the patch.\n> > \n> > I would write the subject as \"Fix compilation warning\" or something\n> > along those lines.\n> \n> I think the subject should state *what* it does, rather than /why/.\n\nYes, but there's always a bit of the 'why' in the 'what'. Otherwise you\ncould write \"remove characters from a line\" :-)\n\nDon't forget that it's common to skim through commit just looking at the\nsubject line, so mentioning the main purpose of the commit there is\nuseful.\n\n> To fix a compilation warning (or error as we use -Werror) is the reason\n> why we fix it :)\n> \n> How about:\n> \n> libcamera: event_dispatcher_poll: Remove struct keyword from for-range\n\nThat works too.\n\n> I'd like to make that 'remove redundant struct keyword' but I've run out\n> of title space :D\n> \n> > On Tue, Jun 25, 2019 at 02:15:08PM +0100, Kieran Bingham wrote:\n> >> GCCv6 and GCCv7 take objections to declaring a struct type when\n> >> using a range based iterator. This issue does not appear in GCCv8\n> >>\n> >>     event_dispatcher_poll.cpp:231:13: error: types may not be defined\n> >> \tin a for-range-declaration [-Werror]\n> >>\n> >> \t\tfor (const struct pollfd &pfd : pollfds) {\n> >> \t\t           ^~~~~~\n> >>\n> >> \tcc1plus: all warnings being treated as errors\n> >>\n> >> Removing the keyword 'struct' ensures that the compiler does not try to\n> >> declare the type, and instead uses the type as already defined by the\n> >> relevant poll.h header.\n> > \n> > I can't reproduce this with gcc 6.4.0 or 7.4.0, I wonder if it could be\n> > related to the C library. You may want to mention the exact compiler\n> > versions that resulted in the error.\n> \n> Using the following code sample, I have tested a range of compilers\n> using godbolt.org:\n> \n> #include <vector>\n> #include <poll.h>\n> \n> void processNotifiers(const std::vector<struct pollfd> &pollfds) {\n>     \tfor (const struct pollfd &pfd : pollfds) {\n> \n>         }\n> }\n> \n> So actually I have misinterpreted the issue. I thought it was a newly\n> introduced compiler warning. In fact, old compilers fail - while newer\n> compilers accept our current syntax.\n> \n> \n> Godbolt shows that this failure occurs at the following versions:\n> \n>  v4.9.3 fails\n>  v4.9.4 fails\n> \n>  v5.1 fails\n>  v5.2 fails\n>  v5.3 fails\n>  v5.4 fails\n>  v5.5 fails\n> \n>  v6.2 fails\n>  v6.3 fails\n> \n>  v6.4 supported\n>  (and so are the versions following)\n> \n> The code sample can be explored at this godbolt short-link:\n>    https://godbolt.org/z/mV6ita\n\nI've tested gcc 5.4.0 here and it doesn't produce that warning :-S I get\na\n\nstruct.cpp: In function ‘void processNotifiers(const std::vector<pollfd>&)’:\nstruct.cpp:6:28: warning: unused variable ‘pfd’ [-Wunused-variable]\n  for (const struct pollfd &pfd : pollfds) {\n                            ^\n\nwhich is expected, and after modifying your sample code to\n\n#include <vector>\n#include <poll.h>\n\nvoid func(const struct pollfd &pfd);\n\nvoid processNotifiers(const std::vector<struct pollfd> &pollfds)\n{\n        for (const struct pollfd &pfd : pollfds) {\n                func(pfd);\n        }\n}\n\nthe code compile cleanly with\n\n$ g++-5.4.0 -W -Wall -std=c++11 -c -o struct struct.cpp\n\nHave you noticed that the error reported by the above godbolt.org link\nfor v5.4 is\n\n<source>: In function 'void processNotifiers(const std::vector<pollfd>&)':\n<source>:5:17: error: types may not be defined in a for-range-declaration [-Werror]\n      for (const struct pollfd &pfd : pollfds) {\n                 ^~~~~~\ncc1plus: all warnings being treated as errors\nCompiler returned: 1\n\nand goes away when you add -std=c++11 ? Only 6.2 and 6.3 seem to suffer\nfrom the struct in range loop issue.\n\n> > In any case,\n> > \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> Thanks, I've updated the commit message a fair bit so I'll send a v2 anyway.\n> --\n> Kieran\n> \n> \n> > \n> >> Reported-by: [autobuild.buildroot.net] Thomas Petazzoni <thomas.petazzoni@bootlin.com>\n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >> ---\n> >>  src/libcamera/event_dispatcher_poll.cpp | 2 +-\n> >>  1 file changed, 1 insertion(+), 1 deletion(-)\n> >>\n> >> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> >> index 0ff99fce47ab..df9dffb2326c 100644\n> >> --- a/src/libcamera/event_dispatcher_poll.cpp\n> >> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> >> @@ -241,7 +241,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol\n> >>  \t\t{ EventNotifier::Exception, POLLPRI },\n> >>  \t};\n> >>  \n> >> -\tfor (const struct pollfd &pfd : pollfds) {\n> >> +\tfor (const pollfd &pfd : pollfds) {\n> >>  \t\tauto iter = notifiers_.find(pfd.fd);\n> >>  \t\tASSERT(iter != notifiers_.end());\n> >>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 BD7C660103\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Jun 2019 19:31:38 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 279B7510;\n\tTue, 25 Jun 2019 19:31:38 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561483898;\n\tbh=LLH+82BoYGj5tC3HQraRr46KRP0qsEXUqnSE0GDaDZ8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PHgAOSBgBGNOi4PW30Miqif/IBQ2OgkrFh5d4LiXOBoozxx8Yrl4UI39RuTN4wJbR\n\tYW/jsv3722I3++FuUJmxMTOP014w+0uOaeL/oq9rviyUxAqa/5i1a+eZp6y7soHg3c\n\t0fVe0Jv1mv0z3uhK9Q3EXWVCYozGmXU1+jQbrKxA=","Date":"Tue, 25 Jun 2019 20:29:07 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>,\n\t\"[autobuild.buildroot.net] Thomas Petazzoni\"\n\t<thomas.petazzoni@bootlin.com>","Message-ID":"<20190625172907.GH5002@pendragon.ideasonboard.com>","References":"<20190625131508.31263-1-kieran.bingham@ideasonboard.com>\n\t<20190625133259.GE5002@pendragon.ideasonboard.com>\n\t<ef5ad6d4-848b-6631-473c-cd0f79e454e2@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<ef5ad6d4-848b-6631-473c-cd0f79e454e2@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 25 Jun 2019 17:31:38 -0000"}},{"id":2026,"web_url":"https://patchwork.libcamera.org/comment/2026/","msgid":"<66ea09a6-dea9-8362-aeb9-9e77ff7c89db@ideasonboard.com>","date":"2019-06-25T20:27:26","subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 25/06/2019 18:29, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Tue, Jun 25, 2019 at 06:13:10PM +0100, Kieran Bingham wrote:\n>> On 25/06/2019 14:32, Laurent Pinchart wrote:\n>>> Hi Kieran,\n>>>\n>>> Thank you for the patch.\n>>>\n>>> I would write the subject as \"Fix compilation warning\" or something\n>>> along those lines.\n>>\n>> I think the subject should state *what* it does, rather than /why/.\n> \n> Yes, but there's always a bit of the 'why' in the 'what'. Otherwise you\n> could write \"remove characters from a line\" :-)\n> \n> Don't forget that it's common to skim through commit just looking at the\n> subject line, so mentioning the main purpose of the commit there is\n> useful.\n> \n>> To fix a compilation warning (or error as we use -Werror) is the reason\n>> why we fix it :)\n>>\n>> How about:\n>>\n>> libcamera: event_dispatcher_poll: Remove struct keyword from for-range\n> \n> That works too.\n> \n>> I'd like to make that 'remove redundant struct keyword' but I've run out\n>> of title space :D\n>>\n>>> On Tue, Jun 25, 2019 at 02:15:08PM +0100, Kieran Bingham wrote:\n>>>> GCCv6 and GCCv7 take objections to declaring a struct type when\n>>>> using a range based iterator. This issue does not appear in GCCv8\n>>>>\n>>>>     event_dispatcher_poll.cpp:231:13: error: types may not be defined\n>>>> \tin a for-range-declaration [-Werror]\n>>>>\n>>>> \t\tfor (const struct pollfd &pfd : pollfds) {\n>>>> \t\t           ^~~~~~\n>>>>\n>>>> \tcc1plus: all warnings being treated as errors\n>>>>\n>>>> Removing the keyword 'struct' ensures that the compiler does not try to\n>>>> declare the type, and instead uses the type as already defined by the\n>>>> relevant poll.h header.\n>>>\n>>> I can't reproduce this with gcc 6.4.0 or 7.4.0, I wonder if it could be\n>>> related to the C library. You may want to mention the exact compiler\n>>> versions that resulted in the error.\n>>\n>> Using the following code sample, I have tested a range of compilers\n>> using godbolt.org:\n>>\n>> #include <vector>\n>> #include <poll.h>\n>>\n>> void processNotifiers(const std::vector<struct pollfd> &pollfds) {\n>>     \tfor (const struct pollfd &pfd : pollfds) {\n>>\n>>         }\n>> }\n>>\n>> So actually I have misinterpreted the issue. I thought it was a newly\n>> introduced compiler warning. In fact, old compilers fail - while newer\n>> compilers accept our current syntax.\n>>\n>>\n>> Godbolt shows that this failure occurs at the following versions:\n>>\n>>  v4.9.3 fails\n>>  v4.9.4 fails\n>>\n>>  v5.1 fails\n>>  v5.2 fails\n>>  v5.3 fails\n>>  v5.4 fails\n>>  v5.5 fails\n>>\n>>  v6.2 fails\n>>  v6.3 fails\n>>\n>>  v6.4 supported\n>>  (and so are the versions following)\n>>\n>> The code sample can be explored at this godbolt short-link:\n>>    https://godbolt.org/z/mV6ita\n> \n> I've tested gcc 5.4.0 here and it doesn't produce that warning :-S I get\n> a\n> \n> struct.cpp: In function ‘void processNotifiers(const std::vector<pollfd>&)’:\n> struct.cpp:6:28: warning: unused variable ‘pfd’ [-Wunused-variable]\n>   for (const struct pollfd &pfd : pollfds) {\n>                             ^\n> \n> which is expected, and after modifying your sample code to\n> \n> #include <vector>\n> #include <poll.h>\n> \n> void func(const struct pollfd &pfd);\n> \n> void processNotifiers(const std::vector<struct pollfd> &pollfds)\n> {\n>         for (const struct pollfd &pfd : pollfds) {\n>                 func(pfd);\n>         }\n> }\n> \n> the code compile cleanly with\n> \n> $ g++-5.4.0 -W -Wall -std=c++11 -c -o struct struct.cpp\n> \n> Have you noticed that the error reported by the above godbolt.org link\n> for v5.4 is\n> \n> <source>: In function 'void processNotifiers(const std::vector<pollfd>&)':\n> <source>:5:17: error: types may not be defined in a for-range-declaration [-Werror]\n>       for (const struct pollfd &pfd : pollfds) {\n>                  ^~~~~~\n> cc1plus: all warnings being treated as errors\n> Compiler returned: 1\n> \n> and goes away when you add -std=c++11 ? Only 6.2 and 6.3 seem to suffer\n> from the struct in range loop issue.\n\nYes, indeed - the -std=c++11 does seem to have an effect here, leaving\nonly 6.2 and 6.3 with issues.\n\nSo we're getting into a very specific compiler corner case :-S\n\nAnyway - best not to dwell on this any longer, this issue has cost way\ntoo much of my time. Lets add a fix and move on, and I'll send an update\nfor the buildroot integration.\n\n--\nRegards\n\nKieran\n\n\n\n>>> In any case,\n>>>\n>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>6563363d1a0665e3f37281d4ce4186f482f70212\n>> Thanks, I've updated the commit message a fair bit so I'll send a v2 anyway.\n>> --\n>> Kieran\n>>\n>>\n>>>\n>>>> Reported-by: [autobuild.buildroot.net] Thomas Petazzoni <thomas.petazzoni@bootlin.com>\n>>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>>> ---\n>>>>  src/libcamera/event_dispatcher_poll.cpp | 2 +-\n>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n>>>> index 0ff99fce47ab..df9dffb2326c 100644\n>>>> --- a/src/libcamera/event_dispatcher_poll.cpp\n>>>> +++ b/src/libcamera/event_dispatcher_poll.cpp\n>>>> @@ -241,7 +241,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol\n>>>>  \t\t{ EventNotifier::Exception, POLLPRI },\n>>>>  \t};\n>>>>  \n>>>> -\tfor (const struct pollfd &pfd : pollfds) {\n>>>> +\tfor (const pollfd &pfd : pollfds) {\n>>>>  \t\tauto iter = notifiers_.find(pfd.fd);\n>>>>  \t\tASSERT(iter != notifiers_.end());\n>>>>  \n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["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 09AFC60BC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Jun 2019 22:27:30 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 96147510;\n\tTue, 25 Jun 2019 22:27:29 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561494449;\n\tbh=MgdmD1Sxn+8UTslxmPRn6Nmc8WSPNt5IxbWL9+Neskk=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=GVB1pleTi48QYe84lrOTuuDnV6Q0jp1vTdHIrpXQn/SONK0Md/FipdGhcye04sIui\n\th/TRmEfu7fit6mJ0fcHKT1QnmMqScXMtAp3uEk/fYU0ekQRxnDDHyiQELawl6g+ijv\n\t/TCmmIOIafRvMm7LKquF9uhTpNhutxhb0d33smiI=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>,\n\t\"[autobuild.buildroot.net] Thomas Petazzoni\"\n\t<thomas.petazzoni@bootlin.com>","References":"<20190625131508.31263-1-kieran.bingham@ideasonboard.com>\n\t<20190625133259.GE5002@pendragon.ideasonboard.com>\n\t<ef5ad6d4-848b-6631-473c-cd0f79e454e2@ideasonboard.com>\n\t<20190625172907.GH5002@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<66ea09a6-dea9-8362-aeb9-9e77ff7c89db@ideasonboard.com>","Date":"Tue, 25 Jun 2019 21:27:26 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190625172907.GH5002@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 25 Jun 2019 20:27:30 -0000"}},{"id":2028,"web_url":"https://patchwork.libcamera.org/comment/2028/","msgid":"<386afade-dd68-888b-01a8-b0e733a887e9@ideasonboard.com>","date":"2019-06-25T20:39:51","subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 25/06/2019 21:27, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> On 25/06/2019 18:29, Laurent Pinchart wrote:\n>> Hi Kieran,\n>>\n>> On Tue, Jun 25, 2019 at 06:13:10PM +0100, Kieran Bingham wrote:\n>>> On 25/06/2019 14:32, Laurent Pinchart wrote:\n>>>> Hi Kieran,\n>>>>\n>>>> Thank you for the patch.\n>>>>\n>>>> I would write the subject as \"Fix compilation warning\" or something\n>>>> along those lines.\n>>>\n>>> I think the subject should state *what* it does, rather than /why/.\n>>\n>> Yes, but there's always a bit of the 'why' in the 'what'. Otherwise you\n>> could write \"remove characters from a line\" :-)\n>>\n>> Don't forget that it's common to skim through commit just looking at the\n>> subject line, so mentioning the main purpose of the commit there is\n>> useful.\n>>\n>>> To fix a compilation warning (or error as we use -Werror) is the reason\n>>> why we fix it :)\n>>>\n>>> How about:\n>>>\n>>> libcamera: event_dispatcher_poll: Remove struct keyword from for-range\n>>\n>> That works too.\n>>\n>>> I'd like to make that 'remove redundant struct keyword' but I've run out\n>>> of title space :D\n>>>\n>>>> On Tue, Jun 25, 2019 at 02:15:08PM +0100, Kieran Bingham wrote:\n>>>>> GCCv6 and GCCv7 take objections to declaring a struct type when\n>>>>> using a range based iterator. This issue does not appear in GCCv8\n>>>>>\n>>>>>     event_dispatcher_poll.cpp:231:13: error: types may not be defined\n>>>>> \tin a for-range-declaration [-Werror]\n>>>>>\n>>>>> \t\tfor (const struct pollfd &pfd : pollfds) {\n>>>>> \t\t           ^~~~~~\n>>>>>\n>>>>> \tcc1plus: all warnings being treated as errors\n>>>>>\n>>>>> Removing the keyword 'struct' ensures that the compiler does not try to\n>>>>> declare the type, and instead uses the type as already defined by the\n>>>>> relevant poll.h header.\n>>>>\n>>>> I can't reproduce this with gcc 6.4.0 or 7.4.0, I wonder if it could be\n>>>> related to the C library. You may want to mention the exact compiler\n>>>> versions that resulted in the error.\n>>>\n>>> Using the following code sample, I have tested a range of compilers\n>>> using godbolt.org:\n>>>\n>>> #include <vector>\n>>> #include <poll.h>\n>>>\n>>> void processNotifiers(const std::vector<struct pollfd> &pollfds) {\n>>>     \tfor (const struct pollfd &pfd : pollfds) {\n>>>\n>>>         }\n>>> }\n>>>\n>>> So actually I have misinterpreted the issue. I thought it was a newly\n>>> introduced compiler warning. In fact, old compilers fail - while newer\n>>> compilers accept our current syntax.\n>>>\n>>>\n>>> Godbolt shows that this failure occurs at the following versions:\n>>>\n>>>  v4.9.3 fails\n>>>  v4.9.4 fails\n>>>\n>>>  v5.1 fails\n>>>  v5.2 fails\n>>>  v5.3 fails\n>>>  v5.4 fails\n>>>  v5.5 fails\n>>>\n>>>  v6.2 fails\n>>>  v6.3 fails\n>>>\n>>>  v6.4 supported\n>>>  (and so are the versions following)\n>>>\n>>> The code sample can be explored at this godbolt short-link:\n>>>    https://godbolt.org/z/mV6ita\n>>\n>> I've tested gcc 5.4.0 here and it doesn't produce that warning :-S I get\n>> a\n>>\n>> struct.cpp: In function ‘void processNotifiers(const std::vector<pollfd>&)’:\n>> struct.cpp:6:28: warning: unused variable ‘pfd’ [-Wunused-variable]\n>>   for (const struct pollfd &pfd : pollfds) {\n>>                             ^\n>>\n>> which is expected, and after modifying your sample code to\n>>\n>> #include <vector>\n>> #include <poll.h>\n>>\n>> void func(const struct pollfd &pfd);\n>>\n>> void processNotifiers(const std::vector<struct pollfd> &pollfds)\n>> {\n>>         for (const struct pollfd &pfd : pollfds) {\n>>                 func(pfd);\n>>         }\n>> }\n>>\n>> the code compile cleanly with\n>>\n>> $ g++-5.4.0 -W -Wall -std=c++11 -c -o struct struct.cpp\n>>\n>> Have you noticed that the error reported by the above godbolt.org link\n>> for v5.4 is\n>>\n>> <source>: In function 'void processNotifiers(const std::vector<pollfd>&)':\n>> <source>:5:17: error: types may not be defined in a for-range-declaration [-Werror]\n>>       for (const struct pollfd &pfd : pollfds) {\n>>                  ^~~~~~\n>> cc1plus: all warnings being treated as errors\n>> Compiler returned:\n1\n>>\n\nAha - I did think it strange that you posted the same error message\nhere, but I seem to have glossed over that fact as a potential copy\npaste error - hence my mis-representing that the same fault occurs\nbefore 6.2...\n\n\nClearly, the message above was supposed to show:\n\n> \n> <source>:5:38: error: range-based 'for' loops only available with -std=c++11 or -std=gnu++11 [-Werror]\n> \n>       for (const struct pollfd &pfd : pollfds) {\n\nWhich clarifies the commit message for the v2.\n\nI'll drop that sentence from the commit and push.\n\n--\nRegards\n\nKieran\n\n\n>> and goes away when you add -std=c++11 ? Only 6.2 and 6.3 seem to suffer\n>> from the struct in range loop issue.\n> \n> Yes, indeed - the -std=c++11 does seem to have an effect here, leaving\n> only 6.2 and 6.3 with issues.\n> \n> So we're getting into a very specific compiler corner case :-S\n> \n> Anyway - best not to dwell on this any longer, this issue has cost way\n> too much of my time. Lets add a fix and move on, and I'll send an update\n> for the buildroot integration.\n> \n> --\n> Regards\n> \n> Kieran\n> \n> \n> \n>>>> In any case,\n>>>>\n>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>> 6563363d1a0665e3f37281d4ce4186f482f70212\n>>> Thanks, I've updated the commit message a fair bit so I'll send a v2 anyway.\n>>> --\n>>> Kieran\n>>>\n>>>\n>>>>\n>>>>> Reported-by: [autobuild.buildroot.net] Thomas Petazzoni <thomas.petazzoni@bootlin.com>\n>>>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>>>> ---\n>>>>>  src/libcamera/event_dispatcher_poll.cpp | 2 +-\n>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n>>>>> index 0ff99fce47ab..df9dffb2326c 100644\n>>>>> --- a/src/libcamera/event_dispatcher_poll.cpp\n>>>>> +++ b/src/libcamera/event_dispatcher_poll.cpp\n>>>>> @@ -241,7 +241,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol\n>>>>>  \t\t{ EventNotifier::Exception, POLLPRI },\n>>>>>  \t};\n>>>>>  \n>>>>> -\tfor (const struct pollfd &pfd : pollfds) {\n>>>>> +\tfor (const pollfd &pfd : pollfds) {\n>>>>>  \t\tauto iter = notifiers_.find(pfd.fd);\n>>>>>  \t\tASSERT(iter != notifiers_.end());\n>>>>>  \n>>\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 61EF560BC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 25 Jun 2019 22:39:54 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C2282510;\n\tTue, 25 Jun 2019 22:39:53 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561495194;\n\tbh=Q4nozelot5xAWwI06ClRe34DuSIgzxMz++VHoIFQhps=;\n\th=Reply-To:Subject:From:To:Cc:References:Date:In-Reply-To:From;\n\tb=qw1WwJVMj/Q4fTqtj81plVeNr4iSU8Pw5lwaIlUjN+0A+JuUOK77Fhz63pZL3HQmN\n\tvB/dZdFNteBW6sG4z/3CPaN2IFGdymJ1xnygcGT/3IqnQ5FmL0TG6cOmYDo9on1g6G\n\t3nADVhXimPoBjiB6b3UYoJbSzhvkRMSOOAkcqu9M=","Reply-To":"kieran.bingham@ideasonboard.com","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>,\n\t\"[autobuild.buildroot.net] Thomas Petazzoni\"\n\t<thomas.petazzoni@bootlin.com>","References":"<20190625131508.31263-1-kieran.bingham@ideasonboard.com>\n\t<20190625133259.GE5002@pendragon.ideasonboard.com>\n\t<ef5ad6d4-848b-6631-473c-cd0f79e454e2@ideasonboard.com>\n\t<20190625172907.GH5002@pendragon.ideasonboard.com>\n\t<66ea09a6-dea9-8362-aeb9-9e77ff7c89db@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<386afade-dd68-888b-01a8-b0e733a887e9@ideasonboard.com>","Date":"Tue, 25 Jun 2019 21:39:51 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<66ea09a6-dea9-8362-aeb9-9e77ff7c89db@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: event_dispatcher_poll:\n\tSimplify range iterator","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 25 Jun 2019 20:39:54 -0000"}}]