Skip to content

Commit 38994a0

Browse files
zhangyichixsysopenci
authored andcommitted
Fixed few static analysis issues<2>
1. Uncaught exception (UNCAUGHT_EXCEPT). 2. Unchecked return value (CHECKED_RETURN). 3. Assigning value MFX_ERR_INVALID_HANDLE to mfx_res here, but that stored value is overwritten before it can be used. 4. Variable c2Handle going out of scope leaks the storage it points to. 5. Dereferencing null pointer outBuffer. Tracked-On: OAM-112437 Signed-off-by: zhangyichix <yichix.zhang@intel.com>
1 parent f4461b5 commit 38994a0

File tree

4 files changed

+38
-20
lines changed

4 files changed

+38
-20
lines changed

c2_store/src/mfx_c2_service.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,11 @@ int main(int /* argc */, char** /* argv */) {
106106
android::hardware::configureRpcThreadpool(8, true /* callerWillJoin */);
107107

108108
RegisterC2Service();
109+
110+
android::hardware::joinRpcThreadpool();
111+
109112
} catch(const std::exception& ex) {
110113
// ALOGE("hardware.intel.media.c2@1.0-service exception: %s", ex.what());
111-
return 0;
112114
}
113-
114-
android::hardware::joinRpcThreadpool();
115115
return 0;
116116
}

c2_utils/src/mfx_gralloc1.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ buffer_handle_t MfxGralloc1Module::ImportBuffer(const buffer_handle_t rawHandle)
280280
buffer_handle_t *outBuffer = nullptr;
281281
int32_t gr1_res = (*m_grImportBufferFunc)(m_gralloc1_dev, rawHandle, outBuffer);
282282

283-
if (GRALLOC1_ERROR_NONE != gr1_res) {
283+
if (GRALLOC1_ERROR_NONE != gr1_res || nullptr == outBuffer) {
284284
MFX_DEBUG_TRACE_I32(gr1_res);
285285
res = C2_BAD_STATE;
286286
}

c2_utils/src/mfx_va_frame_pool_allocator.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include "mfx_c2_utils.h"
2727
#include "mfx_debug.h"
28+
#include "mfx_msdk_debug.h"
2829

2930
#include <C2AllocatorGralloc.h>
3031

@@ -155,6 +156,13 @@ mfxStatus MfxVaFramePoolAllocator::AllocFrames(mfxFrameAllocRequest *request,
155156
}
156157
MFX_DEBUG_TRACE_I32(response->NumFrameActual);
157158

159+
if (MFX_ERR_NONE != mfx_res) {
160+
MFX_DEBUG_TRACE_MSG("Fatal error occurred while allocating memory");
161+
162+
MFX_DEBUG_TRACE__mfxStatus(mfx_res);
163+
return mfx_res;
164+
}
165+
158166
if (response->NumFrameActual >= request->NumFrameMin) {
159167
response->mids = mids.release();
160168
m_pool = std::make_unique<MfxPool<C2GraphicBlock>>(); //release graphic buffer
@@ -172,6 +180,7 @@ mfxStatus MfxVaFramePoolAllocator::AllocFrames(mfxFrameAllocRequest *request,
172180
mfx_res = AllocFrames(request, response);
173181
}
174182

183+
MFX_DEBUG_TRACE__mfxStatus(mfx_res);
175184
return mfx_res;
176185
}
177186

plugin_store/src/mfx_c2_buffer_queue.cpp

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,8 @@ class MfxC2BufferQueueBlockPool::Impl
305305
std::shared_ptr<C2GraphicAllocation> alloc;
306306
c2_status_t err = mAllocator->priorGraphicAllocation(c2Handle, &alloc);
307307
if (err != C2_OK) {
308+
native_handle_close(handle);
309+
native_handle_delete(handle);
308310
return err;
309311
}
310312
std::shared_ptr<MfxC2BufferQueueBlockPoolData> poolData =
@@ -338,17 +340,20 @@ class MfxC2BufferQueueBlockPool::Impl
338340

339341
~Impl() {
340342
bool noInit = false;
341-
for (int i = 0; i < NUM_BUFFER_SLOTS; ++i) {
342-
if (!noInit && mProducer) {
343-
Return<HStatus> transResult =
344-
mProducer->detachBuffer(static_cast<int32_t>(i));
345-
noInit = !transResult.isOk() ||
346-
static_cast<HStatus>(transResult) == HStatus::NO_INIT;
343+
try {
344+
for (int i = 0; i < NUM_BUFFER_SLOTS; ++i) {
345+
if (!noInit && mProducer) {
346+
Return<HStatus> transResult =
347+
mProducer->detachBuffer(static_cast<int32_t>(i));
348+
noInit = !transResult.isOk() ||
349+
static_cast<HStatus>(transResult) == HStatus::NO_INIT;
350+
}
351+
mBuffers[i].clear();
347352
}
348-
mBuffers[i].clear();
353+
gbuffer_.clear();
354+
} catch(const std::exception& e) {
355+
MFX_DEBUG_TRACE_STREAM("Got an exception: " << e.what());
349356
}
350-
351-
gbuffer_.clear();
352357
}
353358

354359
c2_status_t handle(const C2Handle * c2_hdl, buffer_handle_t *hndl){
@@ -570,12 +575,16 @@ class MfxC2BufferQueueBlockPool::Impl
570575
void cancel(uint32_t generation, uint64_t igbp_id, int32_t igbp_slot) {
571576
bool cancelled = false;
572577
{
573-
MFX_DEBUG_TRACE_FUNC;
574-
std::scoped_lock<std::mutex> lock(mMutex);
575-
if (generation == mGeneration && igbp_id == mProducerId && mProducer) {
576-
(void)mProducer->cancelBuffer(igbp_slot, hidl_handle{}).isOk();
577-
cancelled = true;
578-
}
578+
MFX_DEBUG_TRACE_FUNC;
579+
try {
580+
std::scoped_lock<std::mutex> lock(mMutex);
581+
if (generation == mGeneration && igbp_id == mProducerId && mProducer) {
582+
(void)mProducer->cancelBuffer(igbp_slot, hidl_handle{}).isOk();
583+
cancelled = true;
584+
}
585+
} catch(const std::exception& e) {
586+
MFX_DEBUG_TRACE_STREAM("Got an exception: " << e.what());
587+
}
579588
}
580589
}
581590

@@ -632,7 +641,7 @@ MfxC2BufferQueueBlockPoolData::~MfxC2BufferQueueBlockPoolData() {
632641
localPool->cancel(generation, bqId, bqSlot);
633642
}
634643
} else if (igbp && !owner.expired()) {
635-
igbp->cancelBuffer(bqSlot, hidl_handle{}).isOk();
644+
(void)igbp->cancelBuffer(bqSlot, hidl_handle{}).isOk();
636645
}
637646
}
638647

0 commit comments

Comments
 (0)