[libcamera-devel] [PATCH 1/2] android: mm: Null check for CameraBufferManager

[Umang Jain]

Hi Jacopo,

On 10/11/21 3:53 PM, Jacopo Mondi wrote:
> Hi Umang,
>
> On Thu, Oct 07, 2021 at 03:09:36PM +0530, Umang Jain wrote:
>> cros::CameraBufferManager can be nullptr if there is an error in
>> its creation. Place a null-check guard to check it.
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   src/android/mm/cros_camera_buffer.cpp | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
>> index 86770135..97a04c68 100644
>> --- a/src/android/mm/cros_camera_buffer.cpp
>> +++ b/src/android/mm/cros_camera_buffer.cpp
>> @@ -60,6 +60,11 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
>>   	  registered_(false)
>>   {
>>   	bufferManager_ = cros::CameraBufferManager::GetInstance();
>> +	if (!bufferManager_) {
>> +		LOG(HAL, Error)
>> +			<< "Failed to get cros CameraBufferManager instance";
>> +		return;
>> +	}
> I'm not sure if this could happen or is just a "just in case".
> Anyway, the HAL won't be able to operate in that case and this is a
> constructor so it's impossible to propagate the error up to handle it
> gracefully: I would use Fatal.


The "just in case" scenario is well documented in the framework

       // Gets the singleton instance.  Returns nullptr if any error 
occurrs during
       // instance creation.

in 
$chromiumos/src/platform2/camera/include/cros-camera/camera_buffer_manager.h 


I haven't been able to trigger the error condition, but since it's 
document it's better to guard it beforehand.

Fatal sounds good to me too.

>
>>   	int ret = bufferManager_->Register(camera3Buffer);
>>   	if (ret) {
>> --
>> 2.31.1
>>