[{"id":33607,"web_url":"https://patchwork.libcamera.org/comment/33607/","msgid":"","date":"2025-03-17T17:56:09","subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Mar 10, 2025 at 06:02:54PM +0100, Barnabás Pőcze wrote:\n> Only enums whose sizes match that of `int32_t` can be directly\n> supported. Otherwise the expected size and the real size would\n> disagree, leading to an assertion failure in `ControlValue::set()`.\n>\n> Signed-off-by: Barnabás Pőcze \n\nReviewed-by: Jacopo Mondi \n\nThanks!\n\n> ---\n> include/libcamera/controls.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 7c7828ae5..4bfe9615c 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -125,7 +125,7 @@ struct control_type> : public control_type> {\n> };\n>\n> template\n> -struct control_type>> : public control_type {\n> +struct control_type && sizeof(T) == sizeof(int32_t)>> : public control_type {\n\nHowever, for extra paranoia, the controls and properties enumerations\nare unscoped enum without an underlying specified type.\n\nI read that if an underlying type is not specified:\n\"the underlying type is an implementation-defined integral type\"\nhttps://en.cppreference.com/w/cpp/language/enum\n\nwhich makes me wonder if we should go extra paranoid and:\n\n--- a/include/libcamera/control_ids.h.in\n+++ b/include/libcamera/control_ids.h.in\n@@ -31,7 +31,7 @@ namespace {{vendor}} {\n {%- endif %}\n\n {% if ctrls %}\n-enum {\n+enum : int32_t {\n {%- for ctrl in ctrls %}\n {{ctrl.name|snake_case|upper}} = {{ctrl.id}},\n {%- endfor %}\n\n> };\n>\n> } /* namespace details */\n> --\n> 2.48.1\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 62CF7C32F5\n\tfor ;\n\tMon, 17 Mar 2025 17:56:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D66168928;\n\tMon, 17 Mar 2025 18:56:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E91968826\n\tfor ;\n\tMon, 17 Mar 2025 18:56:13 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B3C258DB;\n\tMon, 17 Mar 2025 18:54:31 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"btBVrF0O\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742234071;\n\tbh=VZaBe+3d0t6ypybKj5XWhZeKFA2mqAAuAJ8TrXow06g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=btBVrF0OtEe+1KZujZaH48accuZJJukN6qBsNPe6oTqKxz5WaXQuwEXzA7SoShvSj\n\tYw1rBIPKRWlsAfFA9GhmdhUAG1ILMGx+VOazxMx+TJHGukt9/vgtCYbxjhr/nXVnGK\n\tx7lViVECsVjTEh2pyHIz/JZFWd5HjkLQW/M9qE0M=","Date":"Mon, 17 Mar 2025 18:56:09 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","Message-ID":"","References":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33608,"web_url":"https://patchwork.libcamera.org/comment/33608/","msgid":"<174223420305.3394313.7234656109187812645@ping.linuxembedded.co.uk>","date":"2025-03-17T17:56:43","subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-03-10 17:02:54)\n> Only enums whose sizes match that of `int32_t` can be directly\n> supported. Otherwise the expected size and the real size would\n> disagree, leading to an assertion failure in `ControlValue::set()`.\n> \n> Signed-off-by: Barnabás Pőcze \n\nHas this happened? I assume if we try to make a control from an enum\nwhere we override the type it would now produce a compile failure (which\nI'm fine with for now).\n\nI assume we could make similar mappings of other enum sizes if we needed\nto at that point, but I don't see that as a requirement at the moment.\n\nThis looks positively defensive so:\n\nReviewed-by: Kieran Bingham \n\nWe're in a public header, but I don't think this is an API breaking\nchange at all is it ? If this changed anything we'd see a compile\nfailure already...\n\n\n> ---\n> include/libcamera/controls.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 7c7828ae5..4bfe9615c 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -125,7 +125,7 @@ struct control_type> : public control_type> {\n> };\n> \n> template\n> -struct control_type>> : public control_type {\n> +struct control_type && sizeof(T) == sizeof(int32_t)>> : public control_type {\n> };\n> \n> } /* namespace details */\n> -- \n> 2.48.1\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 86DC4C32F5\n\tfor ;\n\tMon, 17 Mar 2025 17:56:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4240268928;\n\tMon, 17 Mar 2025 18:56:48 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6AB8A68826\n\tfor ;\n\tMon, 17 Mar 2025 18:56:46 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E81918DB;\n\tMon, 17 Mar 2025 18:55:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sE2+U2G2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742234105;\n\tbh=fvFfPKAbp1LQftuChLI5LVNHPN9vrj4eqdDFJm44r30=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=sE2+U2G2wLUFvgIDvefOQp5CEMKuG6EcuhweWs2yE1JMjytzlNL4XQnylKEKe85k3\n\t//uWFtb16ZLDkuS+KPpaC2Awt/if86pTdYx7nekojwOMAOKHuyCUbNd0JSJuvT3F3D\n\tCHP2i34Sx3R37gXn9JBRhJtubulnahOYD6iMWYFg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>","References":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 17 Mar 2025 17:56:43 +0000","Message-ID":"<174223420305.3394313.7234656109187812645@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33649,"web_url":"https://patchwork.libcamera.org/comment/33649/","msgid":"<174238130285.2025019.15955506762767978321@ping.linuxembedded.co.uk>","date":"2025-03-19T10:48:22","subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2025-03-17 17:56:43)\n> Quoting Barnabás Pőcze (2025-03-10 17:02:54)\n> > Only enums whose sizes match that of `int32_t` can be directly\n> > supported. Otherwise the expected size and the real size would\n> > disagree, leading to an assertion failure in `ControlValue::set()`.\n> > \n> > Signed-off-by: Barnabás Pőcze \n> \n> Has this happened? I assume if we try to make a control from an enum\n> where we override the type it would now produce a compile failure (which\n> I'm fine with for now).\n> \n> I assume we could make similar mappings of other enum sizes if we needed\n> to at that point, but I don't see that as a requirement at the moment.\n> \n> This looks positively defensive so:\n> \n> Reviewed-by: Kieran Bingham \n> \n> We're in a public header, but I don't think this is an API breaking\n> change at all is it ? If this changed anything we'd see a compile\n> failure already...\n> \n\ngitlab.freedesktop.org is starting to come back online \\o/\n\nbut alas CI picked something up on this patch...\n\nhttps://gitlab.freedesktop.org/camera/libcamera/-/jobs/73022971\n\nCould you check please? (it could be false positive still or something\nwith the CI)\n\n--\nKieran\n\n> \n> > ---\n> > include/libcamera/controls.h | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 7c7828ae5..4bfe9615c 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -125,7 +125,7 @@ struct control_type> : public control_type> {\n> > };\n> > \n> > template\n> > -struct control_type>> : public control_type {\n> > +struct control_type && sizeof(T) == sizeof(int32_t)>> : public control_type {\n> > };\n> > \n> > } /* namespace details */\n> > -- \n> > 2.48.1\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 B2513C32F5\n\tfor ;\n\tWed, 19 Mar 2025 10:48:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0BE068945;\n\tWed, 19 Mar 2025 11:48:27 +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 8963268942\n\tfor ;\n\tWed, 19 Mar 2025 11:48:25 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 933AA55A;\n\tWed, 19 Mar 2025 11:46:42 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Wu0kUADU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742381202;\n\tbh=pNtaJS2F+dCMp5MgW4FqPrh9KqrBUW1AqxbGukbfP7M=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Wu0kUADUNTGC7c2yWZTeAuXf7BqUd6glM5SdZlOi2oVmjevWX+YVf8tdTlRjCGP8F\n\tVA/StFdr08IQNPdppdbQhStVoqBbVhgIJL11XHuQW5wL58DJgvYbpCFS7N6aVRQ6JZ\n\tKiOjItGM5mxmi2n2ecCxb0TxFBswcuHD1VsYiUno=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<174223420305.3394313.7234656109187812645@ping.linuxembedded.co.uk>","References":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>\n\t<174223420305.3394313.7234656109187812645@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 19 Mar 2025 10:48:22 +0000","Message-ID":"<174238130285.2025019.15955506762767978321@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33650,"web_url":"https://patchwork.libcamera.org/comment/33650/","msgid":"<9ee6e7c6-eac6-451b-aaf6-9205a40b3102@ideasonboard.com>","date":"2025-03-19T12:44:55","subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 03. 19. 11:48 keltezéssel, Kieran Bingham írta:\n> Quoting Kieran Bingham (2025-03-17 17:56:43)\n>> Quoting Barnabás Pőcze (2025-03-10 17:02:54)\n>>> Only enums whose sizes match that of `int32_t` can be directly\n>>> supported. Otherwise the expected size and the real size would\n>>> disagree, leading to an assertion failure in `ControlValue::set()`.\n>>>\n>>> Signed-off-by: Barnabás Pőcze \n>>\n>> Has this happened? I assume if we try to make a control from an enum\n>> where we override the type it would now produce a compile failure (which\n>> I'm fine with for now).\n>>\n>> I assume we could make similar mappings of other enum sizes if we needed\n>> to at that point, but I don't see that as a requirement at the moment.\n>>\n>> This looks positively defensive so:\n>>\n>> Reviewed-by: Kieran Bingham \n>>\n>> We're in a public header, but I don't think this is an API breaking\n>> change at all is it ? If this changed anything we'd see a compile\n>> failure already...\n>>\n> \n> gitlab.freedesktop.org is starting to come back online \\o/\n> \n> but alas CI picked something up on this patch...\n> \n> https://gitlab.freedesktop.org/camera/libcamera/-/jobs/73022971\n> \n> Could you check please? (it could be false positive still or something\n> with the CI)\n\nI am fairly sure that is unrelated.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> --\n> Kieran\n> \n>>\n>>> ---\n>>> include/libcamera/controls.h | 2 +-\n>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>>> index 7c7828ae5..4bfe9615c 100644\n>>> --- a/include/libcamera/controls.h\n>>> +++ b/include/libcamera/controls.h\n>>> @@ -125,7 +125,7 @@ struct control_type> : public control_type> {\n>>> };\n>>> \n>>> template\n>>> -struct control_type>> : public control_type {\n>>> +struct control_type && sizeof(T) == sizeof(int32_t)>> : public control_type {\n>>> };\n>>> \n>>> } /* namespace details */\n>>> -- \n>>> 2.48.1\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 5800AC32FE\n\tfor ;\n\tWed, 19 Mar 2025 12:45:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 776AC6894C;\n\tWed, 19 Mar 2025 13:45:00 +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 D8C2F617F7\n\tfor ;\n\tWed, 19 Mar 2025 13:44:58 +0100 (CET)","from [192.168.33.18] (185.221.143.221.nat.pool.zt.hu\n\t[185.221.143.221])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB08555A;\n\tWed, 19 Mar 2025 13:43:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W3Q/yuAl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742388196;\n\tbh=qWUw+1cSq/3rUpla6MXkyenUDuGivnvbIlMjvU1tbOs=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=W3Q/yuAlMa3dDQTXb/QW8Pyp2cK3MXEfF8cKuWxLRBJTzH5Jn+aE/7qBVIYI3e0Zi\n\tMh0ht9w+DIJTBqAMfgmlHpmCIdiwNFm/5GIQKzvqqIFp6XmH3fcAqAow1BgDU7ohUA\n\tlQyTVZ3bZpwSBRn1C0ahnWdQurueTQRXl0IyrrL4=","Message-ID":"<9ee6e7c6-eac6-451b-aaf6-9205a40b3102@ideasonboard.com>","Date":"Wed, 19 Mar 2025 13:44:55 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>\n\t<174223420305.3394313.7234656109187812645@ping.linuxembedded.co.uk>\n\t<174238130285.2025019.15955506762767978321@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<174238130285.2025019.15955506762767978321@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":33651,"web_url":"https://patchwork.libcamera.org/comment/33651/","msgid":"<174239422241.783128.15746020651374485323@ping.linuxembedded.co.uk>","date":"2025-03-19T14:23:42","subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-03-19 12:44:55)\n> Hi\n> \n> 2025. 03. 19. 11:48 keltezéssel, Kieran Bingham írta:\n> > Quoting Kieran Bingham (2025-03-17 17:56:43)\n> >> Quoting Barnabás Pőcze (2025-03-10 17:02:54)\n> >>> Only enums whose sizes match that of `int32_t` can be directly\n> >>> supported. Otherwise the expected size and the real size would\n> >>> disagree, leading to an assertion failure in `ControlValue::set()`.\n> >>>\n> >>> Signed-off-by: Barnabás Pőcze \n> >>\n> >> Has this happened? I assume if we try to make a control from an enum\n> >> where we override the type it would now produce a compile failure (which\n> >> I'm fine with for now).\n> >>\n> >> I assume we could make similar mappings of other enum sizes if we needed\n> >> to at that point, but I don't see that as a requirement at the moment.\n> >>\n> >> This looks positively defensive so:\n> >>\n> >> Reviewed-by: Kieran Bingham \n> >>\n> >> We're in a public header, but I don't think this is an API breaking\n> >> change at all is it ? If this changed anything we'd see a compile\n> >> failure already...\n> >>\n> > \n> > gitlab.freedesktop.org is starting to come back online \\o/\n> > \n> > but alas CI picked something up on this patch...\n> > \n> > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/73022971\n> > \n> > Could you check please? (it could be false positive still or something\n> > with the CI)\n> \n> I am fairly sure that is unrelated.\n> \n\nAgreed, I re-ran the tests and they've passed, So I'm fine with this\npatch being merged.\n\n--\nKieran\n\n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> > \n> > --\n> > Kieran\n> > \n> >>\n> >>> ---\n> >>> include/libcamera/controls.h | 2 +-\n> >>> 1 file changed, 1 insertion(+), 1 deletion(-)\n> >>>\n> >>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> >>> index 7c7828ae5..4bfe9615c 100644\n> >>> --- a/include/libcamera/controls.h\n> >>> +++ b/include/libcamera/controls.h\n> >>> @@ -125,7 +125,7 @@ struct control_type> : public control_type> {\n> >>> };\n> >>> \n> >>> template\n> >>> -struct control_type>> : public control_type {\n> >>> +struct control_type && sizeof(T) == sizeof(int32_t)>> : public control_type {\n> >>> };\n> >>> \n> >>> } /* namespace details */\n> >>> -- \n> >>> 2.48.1\n> >>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 50F70C3301\n\tfor ;\n\tWed, 19 Mar 2025 14:23:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74ED7687FA;\n\tWed, 19 Mar 2025 15:23:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE9D1687FA\n\tfor ;\n\tWed, 19 Mar 2025 15:23:45 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D32348FA;\n\tWed, 19 Mar 2025 15:22:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PrE46DKp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742394122;\n\tbh=fVi+3BOg0sfaVLRy+wuAP1aVXOUnUAWy0hV2/fnIDFI=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=PrE46DKpdr852rQPIwdPsmfeSwtjyDvLbk8ecxRyoXrUkqjqySKEAFv1QDG75E9y8\n\thGhqt7HjrCieQ3QtUG6zXA8MoSE5XwIidnFAanTzDVhbt7P0fA3LH4XNnphUM4VAnO\n\t0lcTOnQCmgQSpldWqgFHf9DV/4ri+1C9otSMS1/k=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<9ee6e7c6-eac6-451b-aaf6-9205a40b3102@ideasonboard.com>","References":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>\n\t<174223420305.3394313.7234656109187812645@ping.linuxembedded.co.uk>\n\t<174238130285.2025019.15955506762767978321@ping.linuxembedded.co.uk>\n\t<9ee6e7c6-eac6-451b-aaf6-9205a40b3102@ideasonboard.com>","Subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 19 Mar 2025 14:23:42 +0000","Message-ID":"<174239422241.783128.15746020651374485323@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33688,"web_url":"https://patchwork.libcamera.org/comment/33688/","msgid":"<30b8b600-a858-4df4-9fc4-aec5b743fa96@ideasonboard.com>","date":"2025-03-24T09:14:46","subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 03. 17. 18:56 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Mar 10, 2025 at 06:02:54PM +0100, Barnabás Pőcze wrote:\n>> Only enums whose sizes match that of `int32_t` can be directly\n>> supported. Otherwise the expected size and the real size would\n>> disagree, leading to an assertion failure in `ControlValue::set()`.\n>>\n>> Signed-off-by: Barnabás Pőcze \n> \n> Reviewed-by: Jacopo Mondi \n> \n> Thanks!\n> \n>> ---\n>> include/libcamera/controls.h | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index 7c7828ae5..4bfe9615c 100644\n>> --- a/include/libcamera/controls.h\n>> +++ b/include/libcamera/controls.h\n>> @@ -125,7 +125,7 @@ struct control_type> : public control_type> {\n>> };\n>>\n>> template\n>> -struct control_type>> : public control_type {\n>> +struct control_type && sizeof(T) == sizeof(int32_t)>> : public control_type {\n> \n> However, for extra paranoia, the controls and properties enumerations\n> are unscoped enum without an underlying specified type.\n> \n> I read that if an underlying type is not specified:\n> \"the underlying type is an implementation-defined integral type\"\n> https://en.cppreference.com/w/cpp/language/enum\n> \n> which makes me wonder if we should go extra paranoid and:\n> \n> --- a/include/libcamera/control_ids.h.in\n> +++ b/include/libcamera/control_ids.h.in\n> @@ -31,7 +31,7 @@ namespace {{vendor}} {\n> {%- endif %}\n> \n> {% if ctrls %}\n> -enum {\n> +enum : int32_t {\n> {%- for ctrl in ctrls %}\n> {{ctrl.name|snake_case|upper}} = {{ctrl.id}},\n> {%- endfor %}\n> \n>> };\n>>\n>> } /* namespace details */\n\nI think that could be a good idea, maybe something like this:\n\ndiff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\nindex 8f037589d..d56cb08ee 100644\n--- a/include/libcamera/control_ids.h.in\n+++ b/include/libcamera/control_ids.h.in\n@@ -31,7 +31,7 @@ namespace {{vendor}} {\n {%- endif %}\n \n {% if ctrls %}\n-enum {\n+enum : unsigned int {\n {%- for ctrl in ctrls %}\n {{ctrl.name|snake_case|upper}} = {{ctrl.id}},\n {%- endfor %}\n@@ -40,7 +40,7 @@ enum {\n \n {% for ctrl in ctrls -%}\n {% if ctrl.is_enum -%}\n-enum {{ctrl.name}}Enum {\n+enum {{ctrl.name}}Enum : int32_t {\n {%- for enum in ctrl.enum_values %}\n {{enum.name}} = {{enum.value}},\n {%- endfor %}\n\n\nthat is, control identifiers have the underlying type `unsigned int` since that\nis what `ControlList`, etc. use for this purpose, and the specific enumerators\nhave type `int32_t` since that is what `ControlValue`, etc. use.\n\n\nRegards,\nBarnabás Pőcze\n\n\n>> --\n>> 2.48.1\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 F1903C3213\n\tfor ;\n\tMon, 24 Mar 2025 09:14:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C03868951;\n\tMon, 24 Mar 2025 10:14:51 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B8BB617F3\n\tfor ;\n\tMon, 24 Mar 2025 10:14:50 +0100 (CET)","from [192.168.33.23] (185.221.143.221.nat.pool.zt.hu\n\t[185.221.143.221])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7F20EA8F;\n\tMon, 24 Mar 2025 10:13:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OhBEjy0j\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742807583;\n\tbh=xIBB6IWs3AAWJv9EoCyOJQhYd4xiB3FjmFfGSDEimIE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=OhBEjy0jaw9m9mcCnKoY3gB6OUTiK/2wyDA0SCM6uWLOO9ELw96tD6q/Sdd+n+0Ss\n\ttvOaSiCw244VqXwh9WPkZjTwLhbIbhjgG9TJq9+ef+qHjAdrwrhoxvrTk+1S6i+/c7\n\tk0zflDJSfutILEK3DVpDgUJBclfZYuWWMwa2xmaU=","Message-ID":"<30b8b600-a858-4df4-9fc4-aec5b743fa96@ideasonboard.com>","Date":"Mon, 24 Mar 2025 10:14:46 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1] libcamera: controls: Check size of enum","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250310170254.185172-1-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]