[libcamera-devel] [PATCH v3] gstreamer: Add error checking in gst_libcamera_src_task_enter()

[Nicolas Dufresne]

Le mercredi 02 juin 2021 à 02:26 +0530, Vedant Paranjape a écrit :
> Error checking the output from generateConfiguration() was missing in
> the code. Only assert was added as a guard which checked if the size of

Perhaps a little rephrase would help ?

> generated camera config was equal to size of roles passed to it.
> 
> If the roles argument has a invalid member, it will return a nullptr and then
> trying to access a member on a nullptr for size comparison will result in a
> segmentation fault. So, if the function returns a nullptr, it will simply push
> an error message on GstBus and gracefully exit.
> 
> Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>

With better text, you have my:

Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>

> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 87246b40..ccc61590 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -375,10 +375,13 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
>  
>  	/* Generate the stream configurations, there should be one per pad. */
>  	state->config_ = state->cam_->generateConfiguration(roles);
> -	/*
> -	 * \todo Check if camera may increase or decrease the number of streams
> -	 * regardless of the number of roles.
> -	 */
> +	if (state->config_ == nullptr) {
> +		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> +				  ("Failed to generate camera configuration from roles"),
> +				  ("Camera::generateConfiguration() returned nullptr"));
> +		gst_task_stop(task);
> +		return;
> +	}
>  	g_assert(state->config_->size() == state->srcpads_.size());
>  
>  	for (gsize i = 0; i < state->srcpads_.size(); i++) {