[{"id":19404,"web_url":"https://patchwork.libcamera.org/comment/19404/","msgid":"<20210906082421.GP968527@pyrite.rasen.tech>","date":"2021-09-06T08:24:21","subject":"Re: [libcamera-devel] [PATCH] utils: ipc: proxy worker: Fix\n\tindentation in call deserialization","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi,\n\nOn Mon, Sep 06, 2021 at 05:19:08PM +0900, Paul Elder wrote:\n> The indentation of the deserialization call on the proxy worker side\n> inside the case statement was one level too shallow. Fix it.\n> \n> Signed-off-by: Paul Elder \n> ---\n> .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> index c54ecdb9..c306d8df 100644\n> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> @@ -79,7 +79,7 @@ public:\n> \n> {% for method in interface_main.methods %}\n> \t\tcase {{cmd_enum_name}}::{{method.mojom_name|cap}}: {\n> -\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}}\n> +\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}}\n\nNote that this will mix spaces with tabs. But we do so anyway elsewhere\nin the template code. jinja's built-in indent filter (that we use here\nand elsewhere) in 2.x uses spaces only, though in 3.x apparently you can\nspecify the string to indent with. iirc we support both?\n\n\nPaul\n\n> {% for param in method|method_param_outputs %}\n> \t\t\t{{param|name}} {{param.mojom_name}};\n> {% endfor %}\n> -- \n> 2.27.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 421E5BDC71\n\tfor ;\n\tMon, 6 Sep 2021 08:24:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A816A60503;\n\tMon, 6 Sep 2021 10:24:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06AE760503\n\tfor ;\n\tMon, 6 Sep 2021 10:24:28 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE9BA24F\n\tfor ;\n\tMon, 6 Sep 2021 10:24:26 +0200 (CEST)"],"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=\"nfjDC10i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630916667;\n\tbh=zmK1uxLNgXe8/ij0snPaguJiObj9TNlyshU0jo94vtc=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=nfjDC10isgKU39/JXlQgHOBKBlvIr/msOvkUW7THozO8pEY/NcoWffFAXn3RSrb+5\n\tbzKGBMdrihqZBT4xUNoHKFbisPZ+ntS6ie5HzHYX1BFzUUGXwGWXe5LGU9CKxl8bqM\n\tsHLDKuMNXHxsacAWcR/KBMtTdMJobnoQY6OTFx68=","Date":"Mon, 6 Sep 2021 17:24:21 +0900","From":"paul.elder@ideasonboard.com","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20210906082421.GP968527@pyrite.rasen.tech>","References":"<20210906081908.3083134-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210906081908.3083134-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] utils: ipc: proxy worker: Fix\n\tindentation in call deserialization","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19421,"web_url":"https://patchwork.libcamera.org/comment/19421/","msgid":"","date":"2021-09-06T10:46:34","subject":"Re: [libcamera-devel] [PATCH] utils: ipc: proxy worker: Fix\n\tindentation in call deserialization","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 06/09/2021 09:24, paul.elder@ideasonboard.com wrote:\n> Hi,\n> \n> On Mon, Sep 06, 2021 at 05:19:08PM +0900, Paul Elder wrote:\n>> The indentation of the deserialization call on the proxy worker side\n>> inside the case statement was one level too shallow. Fix it.\n>>\n>> Signed-off-by: Paul Elder \n>> ---\n>> .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n>> index c54ecdb9..c306d8df 100644\n>> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n>> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n>> @@ -79,7 +79,7 @@ public:\n>> \n>> {% for method in interface_main.methods %}\n>> \t\tcase {{cmd_enum_name}}::{{method.mojom_name|cap}}: {\n>> -\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}}\n>> +\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}}\n> \n> Note that this will mix spaces with tabs. But we do so anyway elsewhere\n> in the template code. jinja's built-in indent filter (that we use here\n> and elsewhere) in 2.x uses spaces only, though in 3.x apparently you can\n> specify the string to indent with. iirc we support both?\n\nI was going to ask if the indent could be specified in quantified\n'levels' rather than spaces ...\n\nI've seen the mix of spaces and tabs ... it's autogenerated, so I don't\nthink we need to dwell too long over perfection, but any little we can\ndo to help readability is beneficial in these templates.\n\nCertainly reading the output is much easier than reading the templates.\n\nReviewed-by: Kieran Bingham \n\n> \n> \n> Paul\n> \n>> {% for param in method|method_param_outputs %}\n>> \t\t\t{{param|name}} {{param.mojom_name}};\n>> {% endfor %}\n>> -- \n>> 2.27.0\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9559FBDC71\n\tfor ;\n\tMon, 6 Sep 2021 10:46:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 146CC69167;\n\tMon, 6 Sep 2021 12:46:39 +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 9FA4260137\n\tfor ;\n\tMon, 6 Sep 2021 12:46:37 +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 195AA8AD;\n\tMon, 6 Sep 2021 12:46:37 +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=\"oLiGjwuS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630925197;\n\tbh=Nh41yX2CZGhLAMC4GjKU1CWbrddbYz93kdLsXA781kA=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=oLiGjwuScdRN3XJheB12LUn/6NscMNpkHp3cTBr8KQlZwnpoOXnaxKiVjVKuKU3xN\n\tCCZBodhXHWN6JLYY37FdBDz5pqwwMaiV2OwKz0kBGjykHFySGSdCoPjLLsFUBtVXqS\n\tLOtdy1ZchrX3jgzK5S9Bc3ZdX5Ber57C93fvS2Yc=","To":"paul.elder@ideasonboard.com, libcamera-devel@lists.libcamera.org","References":"<20210906081908.3083134-1-paul.elder@ideasonboard.com>\n\t<20210906082421.GP968527@pyrite.rasen.tech>","From":"Kieran Bingham ","Message-ID":"","Date":"Mon, 6 Sep 2021 11:46:34 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210906082421.GP968527@pyrite.rasen.tech>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] utils: ipc: proxy worker: Fix\n\tindentation in call deserialization","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19432,"web_url":"https://patchwork.libcamera.org/comment/19432/","msgid":"","date":"2021-09-06T11:59:47","subject":"Re: [libcamera-devel] [PATCH] utils: ipc: proxy worker: Fix\n\tindentation in call deserialization","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Sep 06, 2021 at 11:46:34AM +0100, Kieran Bingham wrote:\n> On 06/09/2021 09:24, paul.elder@ideasonboard.com wrote:\n> > On Mon, Sep 06, 2021 at 05:19:08PM +0900, Paul Elder wrote:\n> >> The indentation of the deserialization call on the proxy worker side\n> >> inside the case statement was one level too shallow. Fix it.\n> >>\n> >> Signed-off-by: Paul Elder \n> >> ---\n> >> .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +-\n> >> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >>\n> >> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> >> index c54ecdb9..c306d8df 100644\n> >> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> >> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> >> @@ -79,7 +79,7 @@ public:\n> >> \n> >> {% for method in interface_main.methods %}\n> >> \t\tcase {{cmd_enum_name}}::{{method.mojom_name|cap}}: {\n> >> -\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}}\n> >> +\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}}\n> > \n> > Note that this will mix spaces with tabs. But we do so anyway elsewhere\n> > in the template code. jinja's built-in indent filter (that we use here\n> > and elsewhere) in 2.x uses spaces only, though in 3.x apparently you can\n> > specify the string to indent with. iirc we support both?\n> \n> I was going to ask if the indent could be specified in quantified\n> 'levels' rather than spaces ...\n> \n> I've seen the mix of spaces and tabs ... it's autogenerated, so I don't\n> think we need to dwell too long over perfection, but any little we can\n> do to help readability is beneficial in these templates.\n> \n> Certainly reading the output is much easier than reading the templates.\n> \n> Reviewed-by: Kieran Bingham \n\nMy OCD says tabs would be nice, the rest of me says it doesn't matter\nhere :-)\n\nReviewed-by: Laurent Pinchart \n\n> >> {% for param in method|method_param_outputs %}\n> >> \t\t\t{{param|name}} {{param.mojom_name}};\n> >> {% endfor %}","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 064C4BDC71\n\tfor ;\n\tMon, 6 Sep 2021 12:00:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B7F7969167;\n\tMon, 6 Sep 2021 14:00:07 +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 56DE260137\n\tfor ;\n\tMon, 6 Sep 2021 14:00:06 +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 E39B98AD;\n\tMon, 6 Sep 2021 14:00:05 +0200 (CEST)"],"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=\"T9xWnFPm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630929606;\n\tbh=pI4Nr4+2kHs7hi4PN0GvLSBWsdMNVKmKjV/sx0bGknk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=T9xWnFPmWiHpB/gs8c5HZ+GofA9ozVOkqo+a0uykaWAS5HU/zfnSmEBn+eaXCGNhH\n\tvVKFlQf2PI6m4Wohnm3owYrghdbxk/E17FmPoJhtcaMOvpZhkk56wWBlWmlEJbYXPH\n\tmEWLsZq/g9C4tK4S15DYcYGFXwfcUJFLrEbIV4UE=","Date":"Mon, 6 Sep 2021 14:59:47 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210906081908.3083134-1-paul.elder@ideasonboard.com>\n\t<20210906082421.GP968527@pyrite.rasen.tech>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] utils: ipc: proxy worker: Fix\n\tindentation in call deserialization","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]