[libcamera-devel] [PATCH v5 2/7] libcamera: camera: Propagate freeBuffers() error

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-04-16 01:18:54 +0200, Jacopo Mondi wrote:
> The error return code of PipelineHandler::freeBuffers() was not
> propagate up to applications by the Camera class. Fix this by returning
> the incremental error accumulated by multiple calls (one per Stream)
> to freeBuffers().
> 
> Do not return the error code of the call to freeBuffers() in the
> allocateBuffers() method error path not to overwrite the original error
> code returned from the allocation method.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index bdf14b31d8ee..7f2dc904df16 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -671,6 +671,8 @@ int Camera::allocateBuffers()
>   */
>  int Camera::freeBuffers()
>  {
> +	int ret = 0;
> +
>  	if (!stateIs(CameraPrepared))
>  		return -EACCES;
>  
> @@ -683,12 +685,12 @@ int Camera::freeBuffers()
>  		 * by the V4L2 device that has allocated them.
>  		 */
>  		stream->bufferPool().destroyBuffers();
> -		pipe_->freeBuffers(this, stream);
> +		ret |= pipe_->freeBuffers(this, stream);

Or:ing in errors here seems very strange as we are dealing with the 
standard error codes (EINVAL, EBUSY, etc). Would it not be saner to 
store and return the (potential) first error while still running 
freeBuffers() for all streams before returning?

How about something like this

    int ret = 0;

    for (Stream *stream : activeStreams_) {
        int status = pipe_->freeBuffers(this, stream);

        if (!ret)
            ret = status;
    }

    return ret;

>  	}
>  
>  	state_ = CameraConfigured;
>  
> -	return 0;
> +	return ret;
>  }
>  
>  /**
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund