[PATCH v1] libcamera: controls: Check size of enum

[Jacopo Mondi]

Hi Barnabás

On Mon, Mar 10, 2025 at 06:02:54PM +0100, Barnabás Pőcze wrote:
> Only enums whose sizes match that of `int32_t` can be directly
> supported. Otherwise the expected size and the real size would
> disagree, leading to an assertion failure in `ControlValue::set()`.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks!

> ---
>  include/libcamera/controls.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 7c7828ae5..4bfe9615c 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -125,7 +125,7 @@ struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> {
>  };
>
>  template<typename T>
> -struct control_type<T, std::enable_if_t<std::is_enum_v<T>>> : public control_type<int32_t> {
> +struct control_type<T, std::enable_if_t<std::is_enum_v<T> && sizeof(T) == sizeof(int32_t)>> : public control_type<int32_t> {

However, for extra paranoia, the controls and properties enumerations
are unscoped enum without an underlying specified type.

I read that if an underlying type is not specified:
"the underlying type is an implementation-defined integral type"
https://en.cppreference.com/w/cpp/language/enum

which makes me wonder if we should go extra paranoid and:

--- a/include/libcamera/control_ids.h.in
+++ b/include/libcamera/control_ids.h.in
@@ -31,7 +31,7 @@ namespace {{vendor}} {
 {%- endif %}

 {% if ctrls %}
-enum {
+enum : int32_t {
 {%- for ctrl in ctrls %}
        {{ctrl.name|snake_case|upper}} = {{ctrl.id}},
 {%- endfor %}

>  };
>
>  } /* namespace details */
> --
> 2.48.1
>