[PATCH v1 1/2] apps: cam: capture_script: Disallow arrays of strings
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu May 1 10:44:19 CEST 2025
Quoting Barnabás Pőcze (2025-04-22 13:47:50)
> The current `ControlValue` mechanism does not support arrays
> of strings, the assignment in the removed snippet will in fact
> trigger an assertion failure in `ControlValue::set()` because
> `sizeof(std::string) != ControlValueSize[ControlTypeString]`.
>
> Fixes: b35f04b3c194 ("cam: capture_script: Support parsing array controls")
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
I like a clear failure - rather than a guaranteed runtime assertion!
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> src/apps/cam/capture_script.cpp | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp
> index e7e69960e..fdf82efc0 100644
> --- a/src/apps/cam/capture_script.cpp
> +++ b/src/apps/cam/capture_script.cpp
> @@ -578,10 +578,6 @@ ControlValue CaptureScript::parseArrayControl(const ControlId *id,
> value = Span<const float>(values.data(), values.size());
> break;
> }
> - case ControlTypeString: {
> - value = Span<const std::string>(repr.data(), repr.size());
> - break;
> - }
Is it worth keeping
case ControlTypeString:
here as a fall through with a comment explaining that it can't currently
be supported ? (I'm fine with a full removal too - just curious if it's
helpful to document explicitly that it is 'unsupported' rather than
'missed' if someone tried to add it..
> default:
> std::cerr << "Unsupported control type" << std::endl;
> break;
> --
> 2.49.0
>
More information about the libcamera-devel
mailing list