| Message ID | 20250422124401.29968-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2025-04-22 13:44:01) > Arrays of arrays, even arrays of strings, are not supported by > the current `ControlValue` mechanism, so disable them for now > to trigger comptime errors if attempts are made to use them. > This sounds safer than discovering the fault at runtime! Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > 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 326452540..427517a02 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -121,7 +121,7 @@ struct control_type<Point> { > }; > > template<typename T, std::size_t N> > -struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> { > +struct control_type<Span<T, N>, std::enable_if_t<control_type<std::remove_cv_t<T>>::size == 0>> : public control_type<std::remove_cv_t<T>> { > static constexpr std::size_t size = N; > }; > > -- > 2.49.0 >
Hi Barnabás, Thank you for the patch. The subject line should tell what the patch does. On Wed, Apr 23, 2025 at 03:02:14PM +0100, Kieran Bingham wrote: > Quoting Barnabás Pőcze (2025-04-22 13:44:01) > > Arrays of arrays, even arrays of strings, are not supported by > > the current `ControlValue` mechanism, so disable them for now > > to trigger comptime errors if attempts are made to use them. s/comptime/compile-time/ > This sounds safer than discovering the fault at runtime! > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > --- > > 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 326452540..427517a02 100644 > > --- a/include/libcamera/controls.h > > +++ b/include/libcamera/controls.h > > @@ -121,7 +121,7 @@ struct control_type<Point> { > > }; > > > > template<typename T, std::size_t N> > > -struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> { > > +struct control_type<Span<T, N>, std::enable_if_t<control_type<std::remove_cv_t<T>>::size == 0>> : public control_type<std::remove_cv_t<T>> { Maybe with a bit of line wrap, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > static constexpr std::size_t size = N; > > }; > >
Hi 2025. 04. 23. 21:29 keltezéssel, Laurent Pinchart írta: > Hi Barnabás, > > Thank you for the patch. > > The subject line should tell what the patch does. "Disallow arrays of array" ? > > On Wed, Apr 23, 2025 at 03:02:14PM +0100, Kieran Bingham wrote: >> Quoting Barnabás Pőcze (2025-04-22 13:44:01) >>> Arrays of arrays, even arrays of strings, are not supported by >>> the current `ControlValue` mechanism, so disable them for now >>> to trigger comptime errors if attempts are made to use them. > > s/comptime/compile-time/ > >> This sounds safer than discovering the fault at runtime! >> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> --- >>> 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 326452540..427517a02 100644 >>> --- a/include/libcamera/controls.h >>> +++ b/include/libcamera/controls.h >>> @@ -121,7 +121,7 @@ struct control_type<Point> { >>> }; >>> >>> template<typename T, std::size_t N> >>> -struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> { >>> +struct control_type<Span<T, N>, std::enable_if_t<control_type<std::remove_cv_t<T>>::size == 0>> : public control_type<std::remove_cv_t<T>> { > > Maybe with a bit of line wrap, Everything I tried looks bad, do you have something specific in mind? Regards, Barnabás Pőcze > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>> static constexpr std::size_t size = N; >>> }; >>> >
On Fri, Apr 25, 2025 at 10:21:44AM +0200, Barnabás Pőcze wrote: > 2025. 04. 23. 21:29 keltezéssel, Laurent Pinchart írta: > > Hi Barnabás, > > > > Thank you for the patch. > > > > The subject line should tell what the patch does. > > "Disallow arrays of array" ? Ack. > > On Wed, Apr 23, 2025 at 03:02:14PM +0100, Kieran Bingham wrote: > >> Quoting Barnabás Pőcze (2025-04-22 13:44:01) > >>> Arrays of arrays, even arrays of strings, are not supported by > >>> the current `ControlValue` mechanism, so disable them for now > >>> to trigger comptime errors if attempts are made to use them. > > > > s/comptime/compile-time/ > > > >> This sounds safer than discovering the fault at runtime! > >> > >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> > >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >>> --- > >>> 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 326452540..427517a02 100644 > >>> --- a/include/libcamera/controls.h > >>> +++ b/include/libcamera/controls.h > >>> @@ -121,7 +121,7 @@ struct control_type<Point> { > >>> }; > >>> > >>> template<typename T, std::size_t N> > >>> -struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> { > >>> +struct control_type<Span<T, N>, std::enable_if_t<control_type<std::remove_cv_t<T>>::size == 0>> : public control_type<std::remove_cv_t<T>> { > > > > Maybe with a bit of line wrap, > > Everything I tried looks bad, do you have something specific in mind? Maybe struct control_type<Span<T, N>, std::enable_if_t<control_type<std::remove_cv_t<T>>::size == 0>> : public control_type<std::remove_cv_t<T>> { ? It's not a big deal. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > >>> static constexpr std::size_t size = N; > >>> }; > >>>
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 326452540..427517a02 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -121,7 +121,7 @@ struct control_type<Point> { }; template<typename T, std::size_t N> -struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> { +struct control_type<Span<T, N>, std::enable_if_t<control_type<std::remove_cv_t<T>>::size == 0>> : public control_type<std::remove_cv_t<T>> { static constexpr std::size_t size = N; };
Arrays of arrays, even arrays of strings, are not supported by the current `ControlValue` mechanism, so disable them for now to trigger comptime errors if attempts are made to use them. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/controls.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)