Topic: Hidden bugs due to C-style casts

I've been working through a few (thousand) warnings from CppCheck recently, the most common warnings can be summed up as

  • C-style casts - We should always use static_cast<> for converting between classes. C-style casts will automatically do a reinterpret_cast<> when static_cast<> doesn't work, which will definitely lead to broken code (see below)

  • Constructors doesn't initialize all members. This is an obvious advantage, even if they just set values to zero. Use the initializer list if possible.

  • Local scopes for variables. Especially for-loops and such, they should declare their variables locally. I've fixed several bugs caused by for-loop-counters are accessed outside their loops. Local scopes help to break down code

W.r.t to local scopes i mean;

// Prefer local scopes for loops;
for (int i = 1; i <= 3; i++) {
    int n = ...
}
// as opposed to
int i, n;
for (i = 1; i <= 3; i++) {
    n = ...
}

at least w.r.t simple integers, pointers and such. For FloatArrays and such, there are good reasons to define them outside the loops.


For C-style casts, this hides potential errors.

int
PatchIntegrationRule :: SetUpPointsOnTriangle(int nPoints, MaterialMode mode)
{
    numberOfIntegrationPoints = GaussIntegrationRule :: SetUpPointsOnTriangle(nPoints, mode);
    firstLocalStrainIndx = 1;
    lastLocalStrainIndx = 3;
    // convert patch coordinates into element based, update weights accordingly
    for ( int j = 0; j <  numberOfIntegrationPoints; j++ ) {
        GaussPoint *gp = this->gaussPointArray[ j ];
        patch->convertGPIntoParental(gp); // convert coordinates into parental
        Element *elg = patch->giveParent();
        double parentArea = elg->computeArea();
        Triangle *tr = ( Triangle * ) patch; ///@todo This must be a serious bug. This will do a reinterpret_cast from Patch to Triangle, which makes no sense.
        gp->setWeight(8.0 * gp->giveWeight() * tr->getArea() / parentArea); // update integration weight
    }

    return numberOfIntegrationPoints;
}

Triangle and Patch are not compatible. I'm not sure if this was meant to cast to TrianglePatch. This cast will become an reinterpret_cast<> which will only produce a bunch of nonsense results.