Skip to content

Commit

Permalink
Coverity fixes (#110)
Browse files Browse the repository at this point in the history
* Bug fix: the covriance matrix used by some Geometric features was ill-formed

* Various syntax improvements and minor fixes after the Coverity scan

* Syntax cleanup

* Modify the CCVector code syntax + remove warning

* More syntax cleanup

* Allow to cancel the connected components computation at all stages

* Syntax fix
  • Loading branch information
dgirardeau authored Jun 30, 2024
1 parent 2f1cfcf commit dea0b62
Show file tree
Hide file tree
Showing 15 changed files with 239 additions and 125 deletions.
20 changes: 12 additions & 8 deletions include/CCGeom.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,14 @@ template <typename Type> class Vector2Tpl

//! Returns vector square norm
inline Type norm2() const { return (x*x) + (y*y); }
//! Returns vector square norm (forces double precision output)
inline double norm2d() const { return (static_cast<double>(x)*x) + (static_cast<double>(y)*y); }
//! Returns vector norm
inline Type norm() const { return std::sqrt(norm2()); }
inline Type norm() const { return static_cast<Type>(normd()); }
//! Returns vector norm (forces double precision output)
inline double normd() const { return std::sqrt(norm2d()); }
//! Sets vector norm to unity
inline void normalize() { Type n = norm2(); if (n > 0) *this /= std::sqrt(n); }
inline void normalize() { Type n = norm(); if (n >= std::numeric_limits<Type>::epsilon()) *this /= n; }

//! Dot product
inline Type dot(const Vector2Tpl& v) const { return (x*v.x) + (y*v.y); }
Expand Down Expand Up @@ -188,16 +192,16 @@ template <typename Type> class Vector3Tpl : public Tuple3Tpl<Type>
inline Type dot(const Vector3Tpl& v) const { return x*v.x + y*v.y + z*v.z; }
//! Cross product
inline Vector3Tpl cross(const Vector3Tpl &v) const { return Vector3Tpl((y*v.z) - (z*v.y), (z*v.x) - (x*v.z), (x*v.y) - (y*v.x)); }
//! Returns vector square norm
inline Type norm2() const { return x*x + y*y + z*z; }
//! Returns vector square norm (forces double precision output)
inline double norm2d() const { return static_cast<double>(x)*x + static_cast<double>(y)*y + static_cast<double>(z)*z; }
//! Returns vector squared norm
inline Type norm2() const { return (x*x) + (y*y) + (z*z); }
//! Returns vector squared norm (forces double precision output)
inline double norm2d() const { return (static_cast<double>(x)*x) + (static_cast<double>(y)*y) + (static_cast<double>(z)*z); }
//! Returns vector norm
inline Type norm() const { return static_cast<Type>(std::sqrt(norm2d())); }
inline Type norm() const { return static_cast<Type>(normd()); }
//! Returns vector norm (forces double precision output)
inline double normd() const { return std::sqrt(norm2d()); }
//! Sets vector norm to unity
inline void normalize() { Type n = norm(); if (n > std::numeric_limits<Type>::epsilon()) *this /= n; }
inline void normalize() { Type n = norm(); if (n >= std::numeric_limits<Type>::epsilon()) *this /= n; }
//! Returns a normalized vector which is orthogonal to this one
inline Vector3Tpl orthogonal() const { Vector3Tpl ort; vorthogonal(u, ort.u); return ort; }

Expand Down
2 changes: 2 additions & 0 deletions include/DgmOctree.h
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,7 @@ namespace CCCoreLib
- '-1' = no cells (input)
- '-2' = not enough memory
- '-3' = no CC found
- '-4' = process cancelled by user
**/
int extractCCs( const cellCodesContainer& cellCodes,
unsigned char level,
Expand All @@ -1006,6 +1007,7 @@ namespace CCCoreLib
- '-1' = no cells (input)
- '-2' = not enough memory
- '-3' = no CC found
- '-4' = process cancelled by user
**/
int extractCCs( unsigned char level,
bool sixConnexity,
Expand Down
1 change: 1 addition & 0 deletions include/DistanceComputationTools.h
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ namespace CCCoreLib
ERROR_BUILD_OCTREE_FAILURE,
ERROR_BUILD_FAST_MARCHING_FAILURE,
ERROR_UNKOWN_ERRORMEASURES_TYPE,
ERROR_INTERNAL,
INVALID_INPUT,
SUCCESS = 1,
};
Expand Down
7 changes: 6 additions & 1 deletion include/ReferenceCloud.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,12 @@ namespace CCCoreLib
bool add(const ReferenceCloud& cloud);

//! Invalidates the bounding-box
inline void invalidateBoundingBox() { m_bbox.setValidity(false); }
inline void invalidateBoundingBox() { invalidateBoundingBoxInternal(true); }

protected:

//! Invalidates the bounding-box (internal version)
void invalidateBoundingBoxInternal(bool threadSafe);

protected:

Expand Down
136 changes: 84 additions & 52 deletions include/SquareMatrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ namespace CCCoreLib
}
}

//! "From OpenGl" constructor (float version)
//! "From OpenGL" constructor (float version)
/** The matrix dimension is automatically set to 4.
It can be forced to 3 (size_3 = true). In this
case, only the rotation part will be 'imported'.
\param M16f a table of 16 floats (OpenGL float transformation matrix)
\param rotationOnly consider only the roation part (3x3 matrix)
It can be forced to 3 (rotationOnly = true). In this case,
only the rotation part (3x3 upper-left part) will be copied.
\param M16f a table of 16 floats (column-wise 4x4 matrix)
\param rotationOnly to copy only the roation part (3x3 upper-left part of the input matrix)
**/
SquareMatrixTpl(const float M16f[], bool rotationOnly = false)
{
Expand All @@ -78,10 +78,10 @@ namespace CCCoreLib

//! "From OpenGl" constructor (double version)
/** The matrix dimension is automatically set to 4.
It can be forced to 3 (size_3 = true). In this
case, only the rotation part will be 'imported'.
\param M16d a table of 16 floats (OpenGL double transformation matrix)
\param rotationOnly consider only the roation part (3x3 matrix)
It can be forced to 3 (rotationOnly = true). In this case,
only the rotation part (3x3 upper-left part) will be copied.
\param M16d a table of 16 doubles (column-wise 4x4 matrix)
\param rotationOnly to copy only the roation part (3x3 upper-left part of the input matrix)
**/
SquareMatrixTpl(const double M16d[], bool rotationOnly = false)
{
Expand Down Expand Up @@ -114,43 +114,42 @@ namespace CCCoreLib
**/
void invalidate()
{
delete [] m_underlyingData;
m_underlyingData = nullptr;
delete[] m_data;
m_data = nullptr;

delete [] m_values;
delete[] m_values;
m_values = nullptr;

m_matrixSize = 0;
matrixSquareSize = 0;
}

//! The matrix rows
/** public for easy/fast access
**/
Scalar** m_values = nullptr;

//! Returns pointer to matrix row
inline Scalar* row(unsigned index) { return m_values[index]; }

//! Sets a particular matrix value
void inline setValue(unsigned row, unsigned column, Scalar value)
inline void setValue(unsigned row, unsigned column, Scalar value)
{
m_values[row][column] = value;
}

//! Returns a particular matrix value
Scalar inline getValue(unsigned row, unsigned column) const
//! Returns a specific matrix value
inline Scalar getValue(unsigned row, unsigned column) const
{
return m_values[row][column];
}

//! Matrix copy operator
/** \warning Sets an invalid/zero matrix if out of memory
*/
SquareMatrixTpl& operator = (const SquareMatrixTpl& B)
{
if (m_matrixSize != B.size())
{
invalidate();
init(B.size());
if (false == init(B.size()))
{
return *this;
}
}

for (unsigned r = 0; r < m_matrixSize; r++)
Expand Down Expand Up @@ -533,24 +532,30 @@ namespace CCCoreLib
**/
void initFromQuaternion(const float q[])
{
double qd[4] = { static_cast<double>(q[0]),
static_cast<double>(q[1]),
static_cast<double>(q[2]),
static_cast<double>(q[3]) };
double qd[4] { static_cast<double>(q[0]),
static_cast<double>(q[1]),
static_cast<double>(q[2]),
static_cast<double>(q[3]) };

initFromQuaternion(qd);
}

//! Creates a rotation matrix from a quaternion (double version)
/** Quaternion is composed of 4 values: an angle (cos(alpha/2))
and an axis (sin(alpha/2)*unit vector).
\warning the matrix size will be forced to 3x3
\param q normalized quaternion (w,x,y,z)
**/
void initFromQuaternion(const double q[])
{
if (m_matrixSize == 0)
if (!init(3))
return;
if (m_matrixSize != 3)
{
invalidate();
}
if (m_matrixSize == 0 && !init(3))
{
return;
}
assert(m_matrixSize == 3);

double q00 = q[0] * q[0];
Expand All @@ -567,12 +572,12 @@ namespace CCCoreLib
m_values[0][0] = static_cast<Scalar>(q00 + q11 - q22 - q33);
m_values[1][1] = static_cast<Scalar>(q00 - q11 + q22 - q33);
m_values[2][2] = static_cast<Scalar>(q00 - q11 - q22 + q33);
m_values[0][1] = static_cast<Scalar>(2.0*(q12 - q03));
m_values[1][0] = static_cast<Scalar>(2.0*(q12 + q03));
m_values[0][2] = static_cast<Scalar>(2.0*(q13 + q02));
m_values[2][0] = static_cast<Scalar>(2.0*(q13 - q02));
m_values[1][2] = static_cast<Scalar>(2.0*(q23 - q01));
m_values[2][1] = static_cast<Scalar>(2.0*(q23 + q01));
m_values[0][1] = static_cast<Scalar>(2.0 * (q12 - q03));
m_values[1][0] = static_cast<Scalar>(2.0 * (q12 + q03));
m_values[0][2] = static_cast<Scalar>(2.0 * (q13 + q02));
m_values[2][0] = static_cast<Scalar>(2.0 * (q13 - q02));
m_values[1][2] = static_cast<Scalar>(2.0 * (q23 - q01));
m_values[2][1] = static_cast<Scalar>(2.0 * (q23 + q01));
}

//! Converts rotation matrix to quaternion
Expand Down Expand Up @@ -756,10 +761,13 @@ namespace CCCoreLib
}

// Build the output U, S and V matrices
S.init(m_matrixSize);
if ( !S.init(m_matrixSize)
|| !U.init(m_matrixSize)
|| !V.init(m_matrixSize))
{
return false;
}
S.clear();
U.init(m_matrixSize);
V.init(m_matrixSize);
{
// for each eigen value
for (unsigned j = 0; j < m_matrixSize; j++)
Expand Down Expand Up @@ -790,27 +798,47 @@ namespace CCCoreLib
**/
bool init(unsigned size)
{
m_matrixSize = size;
matrixSquareSize = m_matrixSize*m_matrixSize;
if (m_matrixSize == size)
{
// matrix already initialized with the right size
return true;
}
else if (m_matrixSize != 0)
{
// matrix already initialized with the wrong size
invalidate();
}
assert(m_matrixSize == 0);

if ( size == 0 )
if (size == 0)
{
// nothing to do
return true;
}

m_values = new Scalar*[m_matrixSize]{};
m_underlyingData = new Scalar[matrixSquareSize]{};
m_values = new Scalar*[size];
if (nullptr == m_values)
{
// not enough memory
return false;
}

if ( (m_values == nullptr) || (m_underlyingData == nullptr) )
m_data = new Scalar[size * size];
if (nullptr == m_data)
{
// not enough memory
delete[] m_values;
m_values = nullptr;
return false;
}

for (unsigned i = 0; i < m_matrixSize; i++)
for (unsigned i = 0; i < size; i++)
{
m_values[i] = m_underlyingData + (i * m_matrixSize);
m_values[i] = m_data + (i * size);
}

m_matrixSize = size;

return true;
}

Expand All @@ -825,7 +853,7 @@ namespace CCCoreLib
Scalar** subMat = new Scalar*[matSize - 1];
if (subMat)
{
double subDet = 0;
double subDet = 0.0;
double sign = 1.0;

for (unsigned row = 0; row < matSize; row++)
Expand Down Expand Up @@ -1124,16 +1152,20 @@ namespace CCCoreLib
}
}

public: //members

//! The matrix rows
/** public for easy/fast access
**/
Scalar** m_values = nullptr;

private: //members

//! Matrix size
unsigned m_matrixSize;

//! Matrix square-size
unsigned matrixSquareSize;
unsigned m_matrixSize = 0;

//! Stores the actual data, indexed by m_values
Scalar *m_underlyingData = nullptr;
Scalar* m_data = nullptr;
};

//! Default CC square matrix type (PointCoordinateType)
Expand Down
Loading

0 comments on commit dea0b62

Please sign in to comment.