Fix MediaSDK_C2 Coverity issues

issues summary: Uncheck return vaule; self assign; pass nullpointer; Tracked-On: OAM-108581 Signed-off-by: Ke Xiao <[email protected]> Signed-off-by: Zhnang Yichi <[email protected]>
projectceladon · Mar 30, 2023 · f8df2b6 · f8df2b6
1 parent f053e88
commit f8df2b6
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 35 deletions.
diff --git a/c2_components/src/mfx_c2_decoder_component.cpp b/c2_components/src/mfx_c2_decoder_component.cpp
@@ -633,6 +633,7 @@ MfxC2DecoderComponent::MfxC2DecoderComponent(const C2String name, const CreateCo
     // By default prepare buffer to be displayed on any of the common surfaces
     m_consumerUsage = kDefaultConsumerUsage;
 
+    MFX_ZERO_MEMORY(m_signalInfo);
     //m_paramStorage.DumpParams();
 }
 
@@ -797,17 +798,16 @@ c2_status_t MfxC2DecoderComponent::Release()
     }
 #else
     sts = m_mfxSession.Close();
+    if (MFX_ERR_NONE != sts) res = MfxStatusToC2(sts);
 #endif
 
-    if (MFX_ERR_NONE != sts) res = MfxStatusToC2(sts);
 
     if (m_allocator) {
         m_allocator = nullptr;
     }
 
     if (m_device) {
         m_device->Close();
-        if (MFX_ERR_NONE != sts) res = MfxStatusToC2(sts);
 
         m_device = nullptr;
     }
@@ -1814,6 +1814,10 @@ c2_status_t MfxC2DecoderComponent::AllocateFrame(MfxC2FrameOut* frame_out)
         std::unique_ptr<native_handle_t, decltype(hndl_deleter)> hndl(
             android::UnwrapNativeCodec2GrallocHandle(out_block->handle()), hndl_deleter);
 
+        if(hndl == nullptr)
+        {
+            return C2_NO_MEMORY;
+        }
         auto it = m_surfaces.end();
         if (m_mfxVideoParams.IOPattern == MFX_IOPATTERN_OUT_VIDEO_MEMORY) {
 
@@ -1851,7 +1855,7 @@ c2_status_t MfxC2DecoderComponent::AllocateFrame(MfxC2FrameOut* frame_out)
                     m_surfacePool.push_back(frame_out->GetMfxFrameSurface());
                 } else {
                     auto it = m_surfacePool.begin();
-                    for(auto mfx_frame: m_surfacePool) {
+                    for(auto& mfx_frame: m_surfacePool) {
                         // Check if there is avaiable surface in the pool
                         if (!mfx_frame->Data.Locked) {
                             auto blk = m_blocks.begin();

diff --git a/c2_components/src/mfx_c2_encoder_component.cpp b/c2_components/src/mfx_c2_encoder_component.cpp
@@ -459,6 +459,8 @@ MfxC2EncoderComponent::MfxC2EncoderComponent(const C2String name, const CreateCo
     m_codedColorAspects->matrix = C2Color::MATRIX_UNSPECIFIED;
     m_codedColorAspects->primaries = C2Color::PRIMARIES_UNSPECIFIED;
 
+    MFX_ZERO_MEMORY(m_signalInfo);
+    MFX_ZERO_MEMORY(m_mfxInputInfo);
     //m_paramStorage.DumpParams();
 }
 
@@ -1252,6 +1254,7 @@ void MfxC2EncoderComponent::DoWork(std::unique_ptr<C2Work>&& work)
 
             if (MFX_ERR_NONE != mfx_sts) {
                 res = MfxStatusToC2(mfx_sts);
+                MFX_ZERO_MEMORY(mfx_frame_in);
                 break;
             }
 
@@ -1305,8 +1308,8 @@ void MfxC2EncoderComponent::DoWork(std::unique_ptr<C2Work>&& work)
             break;
         }
 
-        m_waitingQueue.Push( [ mfx_frame = std::move(mfx_frame_in), this ] () mutable {
-            RetainLockedFrame(std::move(mfx_frame));
+        m_waitingQueue.Push([&mfx_frame_in, this ] () mutable {
+            RetainLockedFrame(std::move(mfx_frame_in));
         } );
 
         m_pendingWorks.push(std::move(work));
@@ -1877,11 +1880,6 @@ void MfxC2EncoderComponent::DoUpdateMfxParam(const std::vector<C2Param*> &params
             }
             case kParamIndexColorAspects: {
                 if (C2StreamColorAspectsInfo::input::PARAM_TYPE == param->index()) {
-                    m_colorAspects->range = m_colorAspects->range;
-                    m_colorAspects->transfer = m_colorAspects->transfer;
-                    m_colorAspects->matrix = m_colorAspects->matrix;
-                    m_colorAspects->primaries = m_colorAspects->primaries;
-
                     // set video signal info
                     setColorAspects_l();
 
@@ -1890,11 +1888,6 @@ void MfxC2EncoderComponent::DoUpdateMfxParam(const std::vector<C2Param*> &params
                     MFX_DEBUG_TRACE_U32(m_colorAspects->transfer);
                     MFX_DEBUG_TRACE_U32(m_colorAspects->matrix);
                 } else {
-                    m_codedColorAspects->range = m_codedColorAspects->range;
-                    m_codedColorAspects->transfer = m_codedColorAspects->transfer;
-                    m_codedColorAspects->matrix = m_codedColorAspects->matrix;
-                    m_codedColorAspects->primaries = m_codedColorAspects->primaries;
-
                     MFX_DEBUG_TRACE_U32(m_codedColorAspects->range);
                     MFX_DEBUG_TRACE_U32(m_codedColorAspects->primaries);
                     MFX_DEBUG_TRACE_U32(m_codedColorAspects->transfer);
@@ -2102,7 +2095,7 @@ uint32_t MfxC2EncoderComponent::getSyncFramePeriod_l(int32_t sync_frame_period)
         return 0;
     }
 
-    double frame_rate = m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtN / m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtD;
+    double frame_rate = m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtN / m_mfxVideoParamsConfig.mfx.FrameInfo.FrameRateExtD * 1.0;
     double period = sync_frame_period / 1e6 * frame_rate;
 
     MFX_DEBUG_TRACE_F64(frame_rate);

diff --git a/c2_components/src/mfx_c2_vpp_wrapp.cpp b/c2_components/src/mfx_c2_vpp_wrapp.cpp
@@ -40,6 +40,7 @@ MfxC2VppWrapp::MfxC2VppWrapp(void):
     MFX_ZERO_MEMORY(m_vppParam);
     MFX_ZERO_MEMORY(m_allocator);
     MFX_ZERO_MEMORY(m_responses);
+    MFX_ZERO_MEMORY(m_vppSrf);
 }
 
 MfxC2VppWrapp::~MfxC2VppWrapp(void)

diff --git a/c2_store/src/mfx_c2_service.cpp b/c2_store/src/mfx_c2_service.cpp
@@ -93,7 +93,14 @@ int main(int /* argc */, char** /* argv */) {
     ALOGD("[email protected] starting...");
 
     signal(SIGPIPE, SIG_IGN);
-    android::SetUpMinijail(kBaseSeccompPolicyPath, kExtSeccompPolicyPath);
+    try
+    {
+        android::SetUpMinijail(kBaseSeccompPolicyPath, kExtSeccompPolicyPath);
+    }
+    catch(const std::exception& e)
+    {
+        ALOGE("SetUpMinijail function execution error");
+    }
 
     // vndbinder is needed by BufferQueue.
     android::ProcessState::initWithDriver("/dev/vndbinder");

diff --git a/c2_utils/include/mfx_cmd_queue.h b/c2_utils/include/mfx_cmd_queue.h
@@ -22,6 +22,7 @@
 
 #include <C2Work.h>
 
+#include <utils/Log.h>
 #include <functional>
 #include <queue>
 #include <thread>

diff --git a/c2_utils/include/mfx_pool.h b/c2_utils/include/mfx_pool.h
@@ -20,6 +20,7 @@
 
 #pragma once
 
+#include <utils/Log.h>
 #include <mutex>
 #include <future>
 #include <memory>
@@ -46,9 +47,16 @@ class MfxPool
         // Correct method to free pool instance.
         // This waiting for destroyed is needed as module with pool could be unloaded from
         // memory while some resource could try to add itself back to the pool.
-        std::future<void> destroyed = m_poolImpl->Destroyed();
+        try
+        {
+            std::future<void> destroyed = m_poolImpl->Destroyed();
+            destroyed.wait();
+        }
+        catch(const std::exception& e)
+        {
+            // ALOGE("Destroyed function execution error");
+        }
         m_poolImpl.reset();
-        destroyed.wait();
     }
 
     MFX_CLASS_NO_COPY(MfxPool<T>)
@@ -80,7 +88,14 @@ class MfxPool
     public:
         ~MfxPoolImpl()
         {
-            m_destroyed.set_value();
+            try
+            {
+                m_destroyed.set_value();
+            }
+            catch(const std::exception& e)
+            {
+                // ALOGE("set_value function execution error");
+            }
         }
 
         void Append(std::unique_ptr<T>&& free_item)

diff --git a/c2_utils/src/mfx_c2_utils.cpp b/c2_utils/src/mfx_c2_utils.cpp
@@ -669,7 +669,10 @@ YUVWriter::YUVWriter(const std::string& dir,
 
         if (!dir_exists) {
             MFX_DEBUG_TRACE_STREAM(NAMED(full_name.str()));
-            mkdir(full_name.str().c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
+            if(mkdir(full_name.str().c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)) {
+                MFX_DEBUG_TRACE_MSG("cannot create the path");
+                return;
+            }
         }
 
         full_name << "/";

diff --git a/c2_utils/src/mfx_cmd_queue.cpp b/c2_utils/src/mfx_cmd_queue.cpp
@@ -28,7 +28,14 @@
 MfxCmdQueue::~MfxCmdQueue()
 {
     MFX_DEBUG_TRACE(MFX_PTR_NAME(this));
-    Abort();
+    try
+    {
+        Abort();
+    }
+    catch(const std::exception& e)
+    {
+        // ALOGE("Abort function execution error");
+    }
 }
 
 void MfxCmdQueue::Start()

diff --git a/c2_utils/src/mfx_va_allocator.cpp b/c2_utils/src/mfx_va_allocator.cpp
@@ -350,11 +350,22 @@ mfxStatus MfxVaFrameAllocator::UnlockFrame(mfxMemId mid, mfxFrameData *frame_dat
     if (va_mid && va_mid->surface_) {
 
         if (MFX_FOURCC_P8 == va_mid->fourcc_) { // bitstream processing
-            vaUnmapBuffer(m_dpy, *(va_mid->surface_));
-        }
+            VAStatus va_res = vaUnmapBuffer(m_dpy, *(va_mid->surface_));
+            if (VA_STATUS_SUCCESS != va_res) {
+                mfx_res = va_to_mfx_status(va_res);
+                return mfx_res;
+            }
         else { // Image processing
-            vaUnmapBuffer(m_dpy, va_mid->image_.buf);
-            vaDestroyImage(m_dpy, va_mid->image_.image_id);
+            va_res = vaUnmapBuffer(m_dpy, va_mid->image_.buf);
+            if (VA_STATUS_SUCCESS != va_res) {
+                mfx_res = va_to_mfx_status(va_res);
+                return mfx_res;
+            }
+            va_res = vaDestroyImage(m_dpy, va_mid->image_.image_id);
+            if (VA_STATUS_SUCCESS != va_res) {
+                mfx_res = va_to_mfx_status(va_res);
+                return mfx_res;
+            }
 
             if (nullptr != frame_data) {
                 frame_data->Pitch = 0;
@@ -366,6 +377,7 @@ mfxStatus MfxVaFrameAllocator::UnlockFrame(mfxMemId mid, mfxFrameData *frame_dat
         }
     } else {
         mfx_res = MFX_ERR_INVALID_HANDLE;
+        }
     }
 
     MFX_DEBUG_TRACE__mfxStatus(mfx_res);
@@ -628,6 +640,7 @@ mfxStatus MfxVaFrameAllocator::TouchSurface(VASurfaceID surface)
     VAImage image;
     unsigned char* buffer;
     VAStatus va_res;
+    mfxStatus mfx_res = MFX_ERR_NONE;
 
     if (VA_INVALID_ID == surface) return MFX_ERR_UNKNOWN;
 
@@ -638,10 +651,18 @@ mfxStatus MfxVaFrameAllocator::TouchSurface(VASurfaceID surface)
         if (VA_STATUS_SUCCESS == va_res)
         {
             *buffer = 0x0; // can have any value
-            vaUnmapBuffer(m_dpy, image.buf);
+            va_res = vaUnmapBuffer(m_dpy, image.buf);
+            if (VA_STATUS_SUCCESS != va_res) {
+                mfx_res = va_to_mfx_status(va_res);
+                return mfx_res;
+            }
         }
-        vaDestroyImage(m_dpy, image.image_id);
-     }
-    return MFX_ERR_NONE;
+        va_res = vaDestroyImage(m_dpy, image.image_id);
+        if (VA_STATUS_SUCCESS != va_res) {
+            mfx_res = va_to_mfx_status(va_res);
+            return mfx_res;
+        }
+    }
+    return mfx_res;
 }
 #endif // #if defined(LIBVA_SUPPORT)
diff --git a/c2_utils/src/mfx_va_frame_pool_allocator.cpp b/c2_utils/src/mfx_va_frame_pool_allocator.cpp
@@ -134,11 +134,11 @@ mfxStatus MfxVaFramePoolAllocator::AllocFrames(mfxFrameAllocRequest *request,
                 native_handle_t *hndl = android::UnwrapNativeCodec2GrallocHandle(new_block->handle());
                 m_grallocAllocator->GetBackingStore(hndl, &id);
                 m_cachedBufferId.emplace(id, i);
-                if (C2_OK != res) {
-                    native_handle_delete(hndl);
-                    mfx_res = MFX_ERR_MEMORY_ALLOC;
-                    break;
-                }
+                // if (C2_OK != res) { //TODO dead code not need.
+                //     native_handle_delete(hndl);
+                //     mfx_res = MFX_ERR_MEMORY_ALLOC;
+                //     break;
+                // }
 
                 // deep copy to have unique_ptr as m_pool required unique_ptr
                 std::unique_ptr<C2GraphicBlock> unique_block = std::make_unique<C2GraphicBlock>(*new_block);