Skip to content

Commit

Permalink
coverity fixes for external camera and remote camera
Browse files Browse the repository at this point in the history 
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-124500
Signed-off-by: tdivy20 <[email protected]>
Signed-off-by: shivasku82 <[email protected]>
  • Loading branch information
@shivasku82
shivasku82 committed Sep 9, 2024
1 parent 5cd6bfd commit 0007d63
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
4 changes: 4 additions & 0 deletions camera/device/default/ExternalCameraDeviceSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ Status ExternalCameraDeviceSession::switchToOffline(
native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
handle->data[0] = buffer.acquireFence;
outputBuffer.releaseFence = android::makeToAidl(handle);
native_handle_delete(handle);
}
} else {
offlineBuffers.push_back(buffer);
Expand Down Expand Up @@ -1781,6 +1782,7 @@ Status ExternalCameraDeviceSession::processCaptureRequestError(
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);
native_handle_delete(handle);
}
}

Expand Down Expand Up @@ -1827,6 +1829,7 @@ 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);
native_handle_delete(handle);
}
notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER);
} else {
Expand All @@ -1836,6 +1839,7 @@ 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);
native_handle_delete(handle);
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions camera/device/default/ExternalCameraOfflineSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ Status ExternalCameraOfflineSession::processCaptureResult(std::shared_ptr<HalReq
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);
native_handle_delete(handle);
}
notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER);
} else {
Expand All @@ -120,6 +121,7 @@ Status ExternalCameraOfflineSession::processCaptureResult(std::shared_ptr<HalReq
native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
handle->data[0] = req->buffers[i].acquireFence;
outputBuffer.releaseFence = android::makeToAidl(handle);
native_handle_delete(handle);
}
}
}
Expand Down Expand Up @@ -248,6 +250,7 @@ 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);
}
}

Expand Down
17 changes: 14 additions & 3 deletions camera/device/default/RemoteCameraDeviceSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ void *RemoteDataRecvThreadFun(void *argv)
ALOGI(LOG_TAG " %s: Thread is running", __FUNCTION__);
struct pollfd fd;
int event;
uint8_t *fBuffer = nullptr;
uint8_t *fBuffer = nullptr;
RemoteCameraDeviceSession *parentHandle = (RemoteCameraDeviceSession*)argv;
fd.fd = parentHandle->mFd;
fd.events = POLLIN | POLLHUP;
Expand Down Expand Up @@ -163,10 +163,15 @@ 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) {
free(fBuffer);
}
fBuffer = (uint8_t *)malloc(size_pending);
if (fBuffer == NULL) {
ALOGE(LOG_TAG "%s: buffer allocation failed: %d ", __FUNCTION__, __LINE__);
continue;
}

while(true) {
if(mEnqList.size() == 0) {
if (parentHandle->mStopRequest == true) {
Expand Down Expand Up @@ -216,7 +221,6 @@ void *RemoteDataRecvThreadFun(void *argv)
ALOGE("updated fail to convert MJPG to I420 ret %d", res);
}
mDeqList.push_back(frame);
free(fBuffer);
break;
}
}
Expand All @@ -228,7 +232,10 @@ void *RemoteDataRecvThreadFun(void *argv)
}
}
}

if (fBuffer != nullptr) {
free(fBuffer);
fBuffer = nullptr;
}
return argv;
}
RemoteCameraDeviceSession::RemoteCameraDeviceSession(
Expand Down Expand Up @@ -851,6 +858,7 @@ Status RemoteCameraDeviceSession::switchToOffline(
native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
handle->data[0] = buffer.acquireFence;
outputBuffer.releaseFence = android::makeToAidl(handle);
native_handle_delete(handle);
}
} else {
offlineBuffers.push_back(buffer);
Expand Down Expand Up @@ -1683,6 +1691,7 @@ Status RemoteCameraDeviceSession::processCaptureRequestError(
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);
native_handle_delete(handle);
}
}

Expand Down Expand Up @@ -1728,6 +1737,7 @@ Status RemoteCameraDeviceSession::processCaptureResult(std::shared_ptr<HalReques
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);
native_handle_delete(handle);
}
notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER);
} else {
Expand All @@ -1737,6 +1747,7 @@ Status RemoteCameraDeviceSession::processCaptureResult(std::shared_ptr<HalReques
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);
native_handle_delete(handle);
}
}
}
Expand Down
18 changes: 11 additions & 7 deletions camera/provider/default/ExternalCameraProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ bool ExternalCameraProvider::configureCapabilities() {
strerror(errno));
goto out;
}

free(cap_packet);
cap_packet = (camera_packet_t *)malloc(cap_packet_size);
if (cap_packet == NULL) {
ALOGE(LOG_TAG "%s: cap camera_packet_t allocation failed: %d ", __FUNCTION__, __LINE__);
Expand Down Expand Up @@ -351,21 +351,23 @@ void *RemoteThreadFun(void *argv)
ALOGI("Waiting for connection ");
int mSocketServerFd = ::socket(AF_VSOCK, SOCK_STREAM, 0);
if (mSocketServerFd < 0) {
ALOGV(LOG_TAG " %s:Line:[%d] Fail to construct camera socket with error: [%s]",
__FUNCTION__, __LINE__, strerror(errno));
return NULL;
ALOGV(LOG_TAG " %s:Line:[%d] Fail to construct camera socket with error: [%s]",
__FUNCTION__, __LINE__, strerror(errno));
return NULL;
}
ret = ::bind(mSocketServerFd, (struct sockaddr *)&addr_vm,
sizeof(struct sockaddr_vm));
if (ret < 0) {
ALOGV(LOG_TAG " %s Failed to bind port(%d). ret: %d, %s", __func__, addr_vm.svm_port, ret,
strerror(errno));
return NULL;
close(mSocketServerFd);
return NULL;
}
ret = listen(mSocketServerFd, 32);
if (ret < 0) {
ALOGV("%s Failed to listen on ", __FUNCTION__);
return NULL;
ALOGV("%s Failed to listen on ", __FUNCTION__);
close(mSocketServerFd);
return NULL;
}

socklen_t alen = sizeof(struct sockaddr_un);
Expand All @@ -377,7 +379,9 @@ void *RemoteThreadFun(void *argv)
ALOGE("Fail to configure ");
}
}
close(mSocketServerFd);
}

pthread_join(thread_id, NULL);
return argv;
}
Expand Down

0 comments on commit 0007d63

Please sign in to comment.