Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coverity fixes for ExternalCamera and RemoteCamera #170

Open
wants to merge 1 commit into
base: celadon/u/mr0/master
Choose a base branch
from

Conversation

tdivy20
Copy link

@tdivy20 tdivy20 commented Aug 29, 2024

Addressed high priority coverity issues related to resource leaks, string not-null termination etc.

Tests done:

  1. Flashed the image with fixes
  2. Open AOSP camera and Multicamera app
  3. Able to capture and record with both apps

Tracked-On: OAM-122344

camera/device/3.6/default/ExternalCameraDeviceSession.cpp Outdated Show resolved Hide resolved
@@ -798,7 +798,8 @@ Status ExternalCameraDeviceSession::switchToOffline(
native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
handle->data[0] = buffer.acquireFence;
outputBuffer.releaseFence = android::makeToAidl(handle);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct indentation

@@ -1827,7 +1829,8 @@ Status ExternalCameraDeviceSession::processCaptureResult(std::shared_ptr<HalRequ
native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
handle->data[0] = req->buffers[i].acquireFence;
result.outputBuffers[i].releaseFence = ::android::makeToAidl(handle);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct indentation

camera/device/default/ExternalCameraDeviceSession.cpp Outdated Show resolved Hide resolved
@@ -248,7 +250,8 @@ Status ExternalCameraOfflineSession::processCaptureRequestError(
native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
handle->data[0] = req->buffers[i].acquireFence;
outputBuffer.releaseFence = makeToAidl(handle);
}
native_handle_delete(handle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct indentation

@@ -131,10 +131,10 @@ HandleImporter RemoteCameraDeviceSession::sHandleImporter;

void *RemoteDataRecvThreadFun(void *argv)
{
ALOGI(LOG_TAG " %s: Thread is running", __FUNCTION__);
ALOGI(LOG_TAG " %s: divy1 Thread is running", __FUNCTION__);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove your tags

struct pollfd fd;
int event;
uint8_t *fBuffer = nullptr;
uint8_t *fBuffer = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove extra space

@@ -163,10 +163,14 @@ void *RemoteDataRecvThreadFun(void *argv)
size_header = recv(parentHandle->mFd, (char *)&buffer_header, sizeof(camera_header_t), 0);
if(buffer_header.type == CAMERA_DATA){
size_pending = (size_t)buffer_header.size;
if(fBuffer != nullptr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct indentation

if (fBuffer == NULL) {
ALOGE(LOG_TAG "%s: buffer allocation failed: %d ", __FUNCTION__, __LINE__);
}
if (fBuffer == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its better to return if memory allocation fail

Addressed high priority coverity issues related to
resource leaks, string not-null termination etc.

Test done:

1. Flashed the image with fixes
2. Open AOSP camera and Multicamera app
3. Able to capture and record with both apps

Tracked-On: OAM-122344
Signed-off-by: tdivy20 <[email protected]>
Copy link
Contributor

@shivasku82 shivasku82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tdivy20 tdivy20 changed the base branch from master to celadon/u/mr0/master August 29, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants