Scicos, Xcos: fix some Coverity issues 44/17144/1
Clément DAVID [Tue, 1 Sep 2015 12:08:43 +0000 (14:08 +0200)]
Change-Id: I00554a23d9c839a7f095d71481b9e9ef16f0de9e

22 files changed:
scilab/modules/scicos/includes/model/BaseObject.hxx
scilab/modules/scicos/sci_gateway/cpp/sci_buildouttb.cpp
scilab/modules/scicos/sci_gateway/cpp/sci_getscicosvars.cpp
scilab/modules/scicos/sci_gateway/cpp/sci_scicos_log.cpp
scilab/modules/scicos/sci_gateway/cpp/sci_scicosim.cpp
scilab/modules/scicos/src/cpp/LoggerView.cpp
scilab/modules/scicos/src/cpp/Model.cpp
scilab/modules/scicos/src/cpp/vec2var.cpp
scilab/modules/scicos/src/cpp/view_scilab/GraphicsAdapter.cpp
scilab/modules/scicos/src/cpp/view_scilab/LinkAdapter.cpp
scilab/modules/scicos/src/cpp/view_scilab/ModelAdapter.cpp
scilab/modules/xcos/includes/xcosUtilities.hxx
scilab/modules/xcos/sci_gateway/cpp/sci_Xcos.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosDiagramToScilab.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosPalCategoryAdd.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosPalDelete.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosPalDisable.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosPalEnable.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosPalGet.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosPalLoad.cpp
scilab/modules/xcos/sci_gateway/cpp/sci_xcosPalMove.cpp
scilab/modules/xcos/src/cpp/xcosUtilities.cpp

index 91b997c..6b53220 100644 (file)
@@ -33,6 +33,7 @@ public:
         m_id(0), m_kind(b.m_kind)
     {
     }
+    virtual ~BaseObject() = default;
 
     inline bool operator<(BaseObject o) const
     {
index 66b25b1..05b7315 100644 (file)
@@ -312,6 +312,7 @@ types::Function::ReturnValue sci_buildouttb(types::typed_list &in, int _iRetCoun
         if ((n2 == 0) || (m2 == 0))
         {
             out.push_back(pOut);
+            delete[] p1Copy;
             return types::Function::OK;
         }
         else
@@ -319,6 +320,7 @@ types::Function::ReturnValue sci_buildouttb(types::typed_list &in, int _iRetCoun
             Scierror(888, _("%s : inconsistent dimensions between arguments.\n"), funname.data());
             delete[] p1Copy;
             delete[] p2Copy;
+            delete pOut;
             return types::Function::Error;
         }
     }
@@ -327,6 +329,7 @@ types::Function::ReturnValue sci_buildouttb(types::typed_list &in, int _iRetCoun
         Scierror(888, _("%s : bad dimension for argument #%d.\n"), funname.data(), 1);
         delete[] p1Copy;
         delete[] p2Copy;
+        delete pOut;
         return types::Function::Error;
     }
 
@@ -347,6 +350,7 @@ types::Function::ReturnValue sci_buildouttb(types::typed_list &in, int _iRetCoun
             Scierror(888, _("%s : inconsistent dimensions between arguments.\n"), funname.data());
             delete[] p1Copy;
             delete[] p2Copy;
+            delete pOut;
             return types::Function::Error;
         }
     }
@@ -355,6 +359,7 @@ types::Function::ReturnValue sci_buildouttb(types::typed_list &in, int _iRetCoun
         Scierror(888, _("%s : bad dimension for argument #%d.\n"), funname.data(), 2);
         delete[] p1Copy;
         delete[] p2Copy;
+        delete pOut;
         return types::Function::Error;
     }
 
@@ -364,6 +369,7 @@ types::Function::ReturnValue sci_buildouttb(types::typed_list &in, int _iRetCoun
         Scierror(888, _("%s : arguments must have the same length.\n"), funname.data());
         delete[] p1Copy;
         delete[] p2Copy;
+        delete pOut;
         return types::Function::Error;
     }
 
index 8949356..976120d 100644 (file)
@@ -517,6 +517,7 @@ types::Function::ReturnValue sci_getscicosvars(types::typed_list &in, int _iRetC
             Scierror(999, _("%s: Error with parameter \"%s\".\n"), funname.data(), il_str->get(j));
             dyn_char->killMe();
             ret->killMe();
+            FREE(field);
             return types::Function::Error;
         }
 
index d780a5b..5966a5e 100644 (file)
@@ -78,14 +78,13 @@ types::Function::ReturnValue sci_scicos_log(types::typed_list &in, int _iRetCoun
     if (logLevel < 0)
     {
         std::wstringstream buffer;
-        for (int i = 0; i <= LOG_TRACE; i++)
+        for (int i = LOG_TRACE; i < LOG_FATAL; i++)
         {
             buffer << LoggerView::toString(static_cast<enum LogLevel>(i));
-            if (i != LOG_TRACE)
-            {
-                buffer << L", ";
-            }
+            buffer << L", ";
         }
+        buffer << LoggerView::toString(LOG_FATAL);
+
         Scierror(999, _("%s: Wrong value for input argument #%d: Must be in the set  {%ls}.\n"), funame.data(), 1, buffer.str().data());
         return types::Function::Error;
     }
index 3144607..0f22229 100644 (file)
@@ -1277,6 +1277,8 @@ types::Function::ReturnValue sci_scicosim(types::typed_list &in, int _iRetCount,
         il_sim->killMe();
         delete[] il_sim_labptr;
         delete[] l_sim_lab;
+        delete[] il_sim_uidptr;
+        delete[] l_sim_uid;
         return types::Function::Error;
     }
 
@@ -2133,7 +2135,7 @@ types::Function::ReturnValue sci_scicosim(types::typed_list &in, int _iRetCount,
                 break;
 
             case 6  :
-                if ((error = new (std::nothrow) char[MAX_ERROR_LEN]) == nullptr)
+                if ((error = new (std::nothrow) char[MAX_ERROR_LEN]) != nullptr)
                 {
                     allocatedError = TRUE;
                     snprintf(error, MAX_ERROR_LEN, _("the block %d has been called with input out of its domain"), C2F(curblk).kfun);
@@ -2161,7 +2163,7 @@ types::Function::ReturnValue sci_scicosim(types::typed_list &in, int _iRetCount,
                 break;
 
             case 21  :
-                if ((error = new (std::nothrow) char[MAX_ERROR_LEN]) == nullptr)
+                if ((error = new (std::nothrow) char[MAX_ERROR_LEN]) != nullptr)
                 {
                     allocatedError = TRUE;
                     snprintf(error, MAX_ERROR_LEN, _("cannot allocate memory in block=%d"), C2F(curblk).kfun);
@@ -2189,7 +2191,7 @@ types::Function::ReturnValue sci_scicosim(types::typed_list &in, int _iRetCount,
             case 26  :
                 error = _("The number of parameter provided by Scicos blocks is different from what expected by the code generated by the Modelica compiler. You might have relaxed a parameter using FIXED property (i.e., fixed=false) in a Modelica model. This will be corrected in the next version");
                 break;
-                // In this case, you need to turn off the parameter embedded code generation mode by setting 'Modelica_ParEmb=%f' in the Scilab command window, and recompile the Scicos diagram
+            // In this case, you need to turn off the parameter embedded code generation mode by setting 'Modelica_ParEmb=%f' in the Scilab command window, and recompile the Scicos diagram
 
             default  :
                 if (ierr >= 1000)
@@ -2203,7 +2205,7 @@ types::Function::ReturnValue sci_scicosim(types::typed_list &in, int _iRetCount,
                 else if (ierr >= 100)
                 {
                     int istate = -(ierr - 100);
-                    if ((error = new (std::nothrow) char[MAX_ERROR_LEN]) == nullptr)
+                    if ((error = new (std::nothrow) char[MAX_ERROR_LEN]) != nullptr)
                     {
                         allocatedError = TRUE;
                         snprintf(error, MAX_ERROR_LEN, _("integration problem istate=%d"), istate);
index 1cb808d..c7e7077 100644 (file)
@@ -152,6 +152,7 @@ void LoggerView::log(enum LogLevel level, const wchar_t* msg, ...)
     {
         const int N = 1024;
         wchar_t* str = new wchar_t[N];
+
         va_list opts;
         va_start(opts, msg);
         vswprintf(str, N, msg, opts);
@@ -167,6 +168,8 @@ void LoggerView::log(enum LogLevel level, const wchar_t* msg, ...)
             std::cerr << LoggerView::toDisplay(level);
             std::wcerr << str;
         }
+
+        delete[] str;
     }
 }
 
index cfb3ba7..67fe62e 100644 (file)
@@ -164,7 +164,16 @@ void Model::deleteObject(ScicosID uid)
 
 kind_t Model::getKind(ScicosID uid) const
 {
-    return getObject(uid)->kind();
+    model::BaseObject* o = getObject(uid);
+    if (o != nullptr)
+    {
+        return o->kind();
+    }
+    else
+    {
+        // return the first kind, it will be always ignored as the object is no more valid
+        return ANNOTATION;
+    }
 }
 
 std::vector<ScicosID> Model::getAll(kind_t k) const
index a10cc52..23d87ea 100644 (file)
@@ -140,6 +140,7 @@ int decode(const double* const tab, const int tabSize, const int iDims, const in
     if (tabSize < iElements * 2 + 2 + iDims)
     {
         // Error case: the input doesn't have enough elements
+        delete[] pDims;
         Scierror(999, _("%s: Wrong size for input argument #%d: At least %dx%d expected.\n"), vec2varName.c_str(), 1, iElements * 2 + 2 + iDims + offset, 1);
         return -1;
     }
@@ -410,6 +411,7 @@ static bool readElement(const double* const input, const int iType, const int iD
             {
                 if (inputRows < 2 + offset)
                 {
+                    delete pList;
                     Scierror(999, _("%s: Wrong size for input argument #%d: At least %dx%d expected.\n"), vec2varName.c_str(), 1, offset + 2, 1);
                     return false;
                 }
@@ -419,6 +421,7 @@ static bool readElement(const double* const input, const int iType, const int iD
                 types::InternalType* element;
                 if (!readElement(input + offset, elementType, elementDims, inputRows - offset, offset, element))
                 {
+                    delete pList;
                     return false;
                 }
                 pList->append(element);
index 01005e0..1db0b3e 100644 (file)
@@ -254,6 +254,7 @@ struct exprs
         if (v->getType() == types::InternalType::ScilabString)
         {
             types::String* current = v->getAs<types::String>();
+            // FIXME: find a test case and check for this case
         }
         else if (v->getType() == types::InternalType::ScilabDouble)
         {
@@ -306,6 +307,7 @@ struct exprs
                 {
                     types::String* second_string = initial_list->get(1)->getAs<types::String>();
                     // List coming from a "user-defined function" block
+                    // FIXME: why retrieve and not used ?
                 }
                 else if (initial_list->get(1)->getType() == types::InternalType::ScilabList)
                 {
@@ -455,6 +457,7 @@ struct exprs
                         return false;
                     }
                     types::String* last_string = ParamsPDE->get(nParams)->getAs<types::String>();
+                    // FIXME: find a related test case
 
                     // Next comes some code
                     if (initial_list->get(1)->getType() == types::InternalType::ScilabDouble)
@@ -468,6 +471,7 @@ struct exprs
                     else if (initial_list->get(1)->getType() == types::InternalType::ScilabString)
                     {
                         types::String* code = initial_list->get(1)->getAs<types::String>();
+                        // FIXME: find a related test case
                     }
                     else
                     {
@@ -652,7 +656,7 @@ struct exprs
                 return false;
             }
             types::String* outField = current->get(3)->getAs<types::String>();
-            if (!inField->isScalar())
+            if (!outField->isScalar())
             {
                 return false;
             }
@@ -732,6 +736,7 @@ struct exprs
                     return false;
                 }
                 types::String* funtxtField = current->get(9)->getAs<types::String>();
+                // FIXME: find a related test-case
             }
         }
         else
index 2f27e83..6a18ff1 100644 (file)
@@ -295,7 +295,7 @@ link_t getLinkEnd(const LinkAdapter& adaptor, const Controller& controller, cons
 {
     ScicosID adaptee = adaptor.getAdaptee()->id();
 
-    link_t ret; // Initialize a {0, 0, Start} Link
+    link_t ret {0, 0, Start};
 
     ScicosID endID;
     controller.getObjectProperty(adaptee, LINK, end, endID);
index c8bbccc..397bc65 100644 (file)
@@ -1451,10 +1451,13 @@ struct equations
         std::istringstream inputsSizeStr (equations[1]);
         int inputsSize;
         inputsSizeStr >> inputsSize;
-        if (inputsSize == 0)
+        if (inputsSize <= 0 || inputsSize > equations.size() - 2)
         {
             types::Double* inputsField = types::Double::Empty();
             o->set(2, inputsField);
+
+            // fall back to a safe value for future indexing
+            inputsSize = 0;
         }
         else
         {
@@ -1470,10 +1473,13 @@ struct equations
         std::istringstream outputsSizeStr (equations[2 + inputsSize]);
         int outputsSize;
         outputsSizeStr >> outputsSize;
-        if (outputsSize == 0)
+        if (outputsSize <= 0 || outputsSize > equations.size() - 3 - inputsSize)
         {
             types::Double* outputsField = types::Double::Empty();
             o->set(3, outputsField);
+
+            // fall back to a safe value for future indexing
+            outputsSize = 0;
         }
         else
         {
@@ -1492,10 +1498,13 @@ struct equations
         std::istringstream parametersSizeStr (equations[3 + inputsSize + outputsSize]);
         int parametersSize;
         parametersSizeStr >> parametersSize;
-        if (parametersSize == 0)
+        if (parametersSize <= 0 || parametersSize > equations.size() - 4 - inputsSize - outputsSize)
         {
             types::Double* parametersNames = types::Double::Empty();
             parametersField->set(0, parametersNames);
+
+            // fall back to a safe value for future indexing
+            parametersSize = 0;
         }
         else
         {
index 2bfaa4d..49205c8 100644 (file)
@@ -47,4 +47,12 @@ int readSingleString(void* pvApiCtx, int rhsPosition, char** out, const char* fn
  */
 int readVectorString(void* pvApiCtx, int rhsPosition, char*** out, int* vectorLength, char* fname);
 
+/**
+ * Free the allocated vector of strings after the readVectorString call
+ *
+ * @param str the vector of strings
+ * @param len the length of the vector
+ */
+void releaseVectorString(char** str, int len);
+
 #endif /* !__XCOS_UTILITIES_HXX__ */
index fb8c448..d18c75a 100644 (file)
@@ -109,6 +109,7 @@ int sci_Xcos(char *fname, void *pvApiCtx)
         var = (char**) MALLOC(sizeof(char*) * (m * n));
         if (var == NULL)
         {
+            FREE(len);
             Scierror(999, _("%s: No more memory.\n"), fname);
             return 0;
         }
index 59a9115..bfbb405 100644 (file)
@@ -60,7 +60,7 @@ int sci_xcosDiagramToScilab(char *fname, void *pvApiCtx)
         return 0;
     }
 
-    if (iRows1 * iCols1 != 1)
+    if (iRows1 != 1 || iCols1 != 1)
     {
         Scierror(999, _("%s: Can not read input argument #%d.\n"), fname, 1);
     }
@@ -74,7 +74,7 @@ int sci_xcosDiagramToScilab(char *fname, void *pvApiCtx)
         return 0;
     }
 
-    pstXcosFile = (char *)MALLOC(sizeof(char *) * (iLen1 + 1)); //+ 1 for null termination
+    pstXcosFile = (char*) MALLOC(sizeof(char) * (iLen1 + 1)); //+ 1 for null termination
     //get xcos filename
     sciErr = getMatrixOfString(pvApiCtx, piAddr1, &iRows1, &iCols1, &iLen1, &pstXcosFile);
     if (sciErr.iErr)
index f44853a..7faffd2 100644 (file)
@@ -48,6 +48,7 @@ int sci_xcosPalCategoryAdd(char *fname, void* pvApiCtx)
     {
         if (readSingleBoolean(pvApiCtx, 2, &visible, fname))
         {
+            releaseVectorString(name, nameLength);
             return 0;
         }
     }
@@ -64,15 +65,18 @@ int sci_xcosPalCategoryAdd(char *fname, void* pvApiCtx)
     }
     catch (GiwsException::JniCallMethodException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str());
         return 0;
     }
     catch (GiwsException::JniException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str());
         return 0;
     }
 
+    releaseVectorString(name, nameLength);
     PutLhsVar();
     return 0;
 }
index 995a4e2..084caae 100644 (file)
@@ -49,15 +49,18 @@ int sci_xcosPalDelete(char *fname, void* pvApiCtx)
     }
     catch (GiwsException::JniCallMethodException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str());
         return 0;
     }
     catch (GiwsException::JniException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str());
         return 0;
     }
 
+    releaseVectorString(name, nameLength);
     PutLhsVar();
     return 0;
 }
index bb4458f..bb35c7e 100644 (file)
@@ -49,15 +49,19 @@ int sci_xcosPalDisable(char *fname, void* pvApiCtx)
     }
     catch (GiwsException::JniCallMethodException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str());
         return 0;
     }
     catch (GiwsException::JniException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str());
         return 0;
     }
 
+    releaseVectorString(name, nameLength);
     PutLhsVar();
     return 0;
 }
+
index 13869a2..b0968ee 100644 (file)
@@ -50,15 +50,18 @@ int sci_xcosPalEnable(char *fname, void* pvApiCtx)
     }
     catch (GiwsException::JniCallMethodException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str());
         return 0;
     }
     catch (GiwsException::JniException &exception)
     {
+        releaseVectorString(name, nameLength);
         Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str());
         return 0;
     }
 
+    releaseVectorString(name, nameLength);
     PutLhsVar();
     return 0;
 }
index 942a898..cc820cb 100644 (file)
@@ -49,15 +49,18 @@ int sci_xcosPalGet(char *fname, void* pvApiCtx)
     }
     catch (GiwsException::JniCallMethodException &exception)
     {
+        releaseVectorString(category, lenCategory);
         Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str());
         return 0;
     }
     catch (GiwsException::JniException &exception)
     {
+        releaseVectorString(category, lenCategory);
         Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str());
         return 0;
     }
 
+    releaseVectorString(category, lenCategory);
     PutLhsVar();
     return 0;
 }
index dbd096b..51b9037 100644 (file)
@@ -67,15 +67,30 @@ int sci_xcosPalLoad(char *fname, void* pvApiCtx)
     }
     catch (GiwsException::JniCallMethodException &exception)
     {
+        if (category != NULL)
+        {
+            releaseVectorString(category, lenCategory);
+        }
+        FREE(name);
         Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str());
         return 0;
     }
     catch (GiwsException::JniException &exception)
     {
+        if (category != NULL)
+        {
+            releaseVectorString(category, lenCategory);
+        }
+        FREE(name);
         Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str());
         return 0;
     }
 
+    if (category != NULL)
+    {
+        releaseVectorString(category, lenCategory);
+    }
+    FREE(name);
     PutLhsVar();
     return 0;
 }
index 2e4a6c1..1f9dd6b 100644 (file)
@@ -47,6 +47,7 @@ int sci_xcosPalMove(char *fname, void* pvApiCtx)
     /* target setup */
     if (readVectorString(pvApiCtx, 2, &target, &targetLength, fname))
     {
+        releaseVectorString(target, targetLength);
         return 0;
     }
 
@@ -58,15 +59,21 @@ int sci_xcosPalMove(char *fname, void* pvApiCtx)
     }
     catch (GiwsException::JniCallMethodException &exception)
     {
+        releaseVectorString(source, sourceLength);
+        releaseVectorString(target, targetLength);
         Scierror(999, "%s: %s\n", fname, exception.getJavaDescription().c_str());
         return 0;
     }
     catch (GiwsException::JniException &exception)
     {
+        releaseVectorString(source, sourceLength);
+        releaseVectorString(target, targetLength);
         Scierror(999, "%s: %s\n", fname, exception.whatStr().c_str());
         return 0;
     }
 
+    releaseVectorString(source, sourceLength);
+    releaseVectorString(target, targetLength);
     PutLhsVar();
     return 0;
 }
index 1d4537a..21dc765 100644 (file)
@@ -172,3 +172,12 @@ int readVectorString(void* pvApiCtx, int rhsPosition, char*** out, int* vectorLe
     return 0;
 }
 
+void releaseVectorString(char** str, int len)
+{
+    for (int i = 0; i < len; ++i)
+    {
+        FREE(str[i]);
+    }
+    FREE(str);
+}
+