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

[Kieran Bingham]

Quoting Barnabás Pőcze (2025-03-19 12:44:55)
> Hi
> 
> 2025. 03. 19. 11:48 keltezéssel, Kieran Bingham írta:
> > Quoting Kieran Bingham (2025-03-17 17:56:43)
> >> Quoting Barnabás Pőcze (2025-03-10 17:02:54)
> >>> 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>
> >>
> >> Has this happened? I assume if we try to make a control from an enum
> >> where we override the type it would now produce a compile failure (which
> >> I'm fine with for now).
> >>
> >> I assume we could make similar mappings of other enum sizes if we needed
> >> to at that point, but I don't see that as a requirement at the moment.
> >>
> >> This looks positively defensive so:
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >>
> >> We're in a public header, but I don't think this is an API breaking
> >> change at all is it ? If this changed anything we'd see a compile
> >> failure already...
> >>
> > 
> > gitlab.freedesktop.org is starting to come back online \o/
> > 
> > but alas CI picked something up on this patch...
> > 
> > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/73022971
> > 
> > Could you check please? (it could be false positive still or something
> > with the CI)
> 
> I am fairly sure that is unrelated.
> 

Agreed, I re-ran the tests and they've passed, So I'm fine with this
patch being merged.

--
Kieran

> 
> Regards,
> Barnabás Pőcze
> 
> 
> > 
> > --
> > Kieran
> > 
> >>
> >>> ---
> >>>   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> {
> >>>   };
> >>>   
> >>>   } /* namespace details */
> >>> -- 
> >>> 2.48.1
> >>>
>