[PATCH v1] apps: cam: capture_script: Simplify bool array parsing

[Jacopo Mondi]

Hi Barnabás

On Thu, Apr 17, 2025 at 01:33:42PM +0200, Barnabás Pőcze wrote:
> `std::vector<bool>` is a specialization that implements a dynamic
> bit vector, therefore it is not suitable to provide storage for
> an array of `bool`. Hence a statically sized array is used when
> parsing an array of boolean values.
>
> Instead, use the array overload of `std::make_unique` since the
> size is known beforehand.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  src/apps/cam/capture_script.cpp | 36 +++++++--------------------------
>  1 file changed, 7 insertions(+), 29 deletions(-)
>
> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp
> index 7d079721a..8ecd89f28 100644
> --- a/src/apps/cam/capture_script.cpp
> +++ b/src/apps/cam/capture_script.cpp
> @@ -8,6 +8,7 @@
>  #include "capture_script.h"
>
>  #include <iostream>
> +#include <memory>
>  #include <stdio.h>
>  #include <stdlib.h>
>
> @@ -521,45 +522,22 @@ ControlValue CaptureScript::parseArrayControl(const ControlId *id,
>  	case ControlTypeNone:
>  		break;
>  	case ControlTypeBool: {
> -		/*
> -		 * This is unpleasant, but we cannot use an std::vector<> as its
> -		 * boolean type overload does not allow to access the raw data,
> -		 * as boolean values are stored in a bitmask for efficiency.
> -		 *
> -		 * As we need a contiguous memory region to wrap in a Span<>,
> -		 * use an array instead but be strict about not overflowing it
> -		 * by limiting the number of controls we can store.
> -		 *
> -		 * Be loud but do not fail, as the issue would present at
> -		 * runtime and it's not fatal.
> -		 */
> -		static constexpr unsigned int kMaxNumBooleanControls = 1024;
> -		std::array<bool, kMaxNumBooleanControls> values;
> -		unsigned int idx = 0;
> +		auto values = std::make_unique<bool[]>(repr.size());

Brilliant, much better than fixed-size arrays

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


>
> -		for (const std::string &s : repr) {
> -			bool val;
> +		for (std::size_t i = 0; i < repr.size(); i++) {
> +			const auto &s = repr[i];
>
>  			if (s == "true") {
> -				val = true;
> +				values[i] = true;
>  			} else if (s == "false") {
> -				val = false;
> +				values[i] = false;
>  			} else {
>  				unpackFailure(id, s);
>  				return value;
>  			}
> -
> -			if (idx == kMaxNumBooleanControls) {
> -				std::cerr << "Cannot parse more than "
> -					  << kMaxNumBooleanControls
> -					  << " boolean controls" << std::endl;
> -				break;
> -			}
> -
> -			values[idx++] = val;
>  		}
>
> -		value = Span<bool>(values.data(), idx);
> +		value = Span<bool>(values.get(), repr.size());
>  		break;
>  	}
>  	case ControlTypeByte: {
> --
> 2.49.0
>