[v2,1/3] apps: cam: capture_script: Disallow arrays of strings
diff mbox series

Message ID 20250501095818.3996419-2-barnabas.pocze@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: controls: String controls as `std::string_view`
Related show

Commit Message

Barnabás Pőcze May 1, 2025, 9:58 a.m. UTC 
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@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/apps/cam/capture_script.cpp | 4 ----
 1 file changed, 4 deletions(-)

Comments

Jacopo Mondi May 2, 2025, 9:25 a.m. UTC | #1 
Hi Barnabás

On Thu, May 01, 2025 at 11:58:16AM +0200, Barnabás Pőcze wrote:
> 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@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

Thanks
  j

> ---
>  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;
> -	}
>  	default:
>  		std::cerr << "Unsupported control type" << std::endl;
>  		break;
> --
> 2.49.0
>
Paul Elder May 13, 2025, 3:01 p.m. UTC | #2 
Quoting Barnabás Pőcze (2025-05-01 11:58:16)
> 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@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@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;
> -       }
>         default:
>                 std::cerr << "Unsupported control type" << std::endl;
>                 break;
> -- 
> 2.49.0
>
Laurent Pinchart Oct. 2, 2025, 2:52 p.m. UTC | #3 
Hi Barnabás,

Thank you for the patch.

On Thu, May 01, 2025 at 11:58:16AM +0200, Barnabás Pőcze wrote:
> 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@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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;
> -	}
>  	default:
>  		std::cerr << "Unsupported control type" << std::endl;
>  		break;
Barnabás Pőcze Oct. 2, 2025, 2:56 p.m. UTC | #4 
2025. 10. 02. 16:52 keltezéssel, Laurent Pinchart írta:
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Thu, May 01, 2025 at 11:58:16AM +0200, Barnabás Pőcze wrote:
>> 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@ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The first two patches in this series are already merged, I am afraid. :(


> 
>> ---
>>   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;
>> -	}
>>   	default:
>>   		std::cerr << "Unsupported control type" << std::endl;
>>   		break;
>

Patch
diff mbox series 

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;
-	}
 	default:
 		std::cerr << "Unsupported control type" << std::endl;
 		break;