Fix warnings found by PVS

- 1 copy-paste error
  - 1 nullptr dereference
  - fixed self-assignment in Mesh::VertexDescription::operator=
  - a few missing delete
  - all other are checks for nullptr after new
pull/110/head
Hleb Valoshka 2018-08-09 18:25:54 +03:00
parent fa5ab79304
commit 1fda8e35f0
28 changed files with 82 additions and 196 deletions

View File

@ -61,6 +61,7 @@ Body::~Body()
delete s.second;
delete altSurfaces;
}
delete locations;
}

View File

@ -172,15 +172,13 @@ const string Constellation::getAbbreviation() const
void Constellation::initialize()
{
int nConstellations = sizeof(constellationInfo) / sizeof(constellationInfo[0]);
// XXX: replace with std::array or std::vector
constellations = new Constellation* [nConstellations];
if (constellations != nullptr)
for (int i = 0; i < nConstellations; i++)
{
for (int i = 0; i < nConstellations; i++)
{
constellations[i] = new Constellation(constellationInfo[i].name,
constellationInfo[i].gen,
constellationInfo[i].abbr);
}
constellations[i] = new Constellation(constellationInfo[i].name,
constellationInfo[i].gen,
constellationInfo[i].abbr);
}
}

View File

@ -198,9 +198,6 @@ Image* LoadDDSImage(const string& filename)
(int) ddsd.width,
(int) ddsd.height,
max(ddsd.mipMapLevels, 1u));
if (img == nullptr)
return nullptr;
in.read(reinterpret_cast<char*>(img->getPixels()), img->getSize());
if (!in.eof() && !in.good())
{

View File

@ -73,18 +73,12 @@ string DeepSkyObject::getDescription() const
string DeepSkyObject::getInfoURL() const
{
if (infoURL == nullptr)
return "";
else
return *infoURL;
return infoURL;
}
void DeepSkyObject::setInfoURL(const string& s)
{
if (infoURL == nullptr)
infoURL = new string(s);
else
*infoURL = s;
infoURL = s;
}

View File

@ -19,7 +19,7 @@
#include <Eigen/Core>
#include <Eigen/Geometry>
const float DSO_DEFAULT_ABS_MAGNITUDE = -1000.0f;
constexpr const float DSO_DEFAULT_ABS_MAGNITUDE = -1000.0f;
class Nebula;
class Galaxy;
@ -100,11 +100,11 @@ class DeepSkyObject
private:
uint32_t catalogNumber{ InvalidCatalogNumber };
Eigen::Vector3d position{ 0, 0, 0 };
Eigen::Vector3d position{ Eigen::Vector3d::Zero() };
Eigen::Quaternionf orientation{ Eigen::Quaternionf::Identity() };
float radius{ 1 };
float absMag{ DSO_DEFAULT_ABS_MAGNITUDE } ;
std::string* infoURL{ nullptr };
std::string infoURL;
bool visible { true };
bool clickable { true };

View File

@ -287,28 +287,23 @@ bool DSODatabase::load(istream& in, const string& resourcePath)
// assumption here is that there will be small numbers of
// DSOs in text files added to a big collection loaded from
// a binary file.
capacity = (int) (capacity * 1.05);
capacity = (int) (capacity * 1.05);
// 100 DSOs seems like a reasonable minimum
if (capacity < 100)
capacity = 100;
capacity = 100;
DeepSkyObject** newDSOs = new DeepSkyObject*[capacity];
if (newDSOs == nullptr)
{
DPRINTF(0, "Out of memory!");
return false;
}
DeepSkyObject** newDSOs = new DeepSkyObject*[capacity];
if (DSOs != nullptr)
{
copy(DSOs, DSOs + nDSOs, newDSOs);
delete[] DSOs;
}
DSOs = newDSOs;
DSOs = newDSOs;
}
DSOs[nDSOs++] = obj;
DSOs[nDSOs++] = obj;
obj->setCatalogNumber(objCatalogNumber);
@ -328,7 +323,7 @@ bool DSODatabase::load(istream& in, const string& resourcePath)
string::size_type length = string::npos;
if (next != string::npos)
{
length = next - startPos;
length = next - startPos;
++next;
}
string DSOName = objName.substr(startPos, length);

View File

@ -183,8 +183,6 @@ GLShaderLoader::CreateVertexShader(const vector<string>& source,
GLuint vsid = glCreateShader(GL_VERTEX_SHADER);
auto* shader = new GLVertexShader(vsid);
if (!shader)
return ShaderStatus_OutOfMemory;
GLShaderStatus status = shader->compile(source);
if (status != ShaderStatus_OK)
@ -211,8 +209,6 @@ GLShaderLoader::CreateFragmentShader(const vector<string>& source,
GLuint fsid = glCreateShader(GL_FRAGMENT_SHADER);
auto* shader = new GLFragmentShader(fsid);
if (!shader)
return ShaderStatus_OutOfMemory;
GLShaderStatus status = shader->compile(source);
if (status != ShaderStatus_OK)
@ -261,8 +257,6 @@ GLShaderLoader::CreateProgram(const GLVertexShader& vs,
GLuint progid = glCreateProgram();
auto* prog = new GLProgram(progid);
if (!prog)
return ShaderStatus_OutOfMemory;
prog->attach(vs);
prog->attach(fs);
@ -335,8 +329,6 @@ GetInfoLog(GLuint obj)
return string();
auto* log = new char[logLength];
if (!log)
return string();
glGetShaderInfoLog(obj, logLength, &charsWritten, log);

View File

@ -302,8 +302,6 @@ Image* Image::computeNormalMap(float scale, bool wrap) const
return nullptr;
auto* normalMap = new Image(GL_RGBA, width, height);
if (!normalMap)
return nullptr;
unsigned char* nmPixels = normalMap->getPixels();
int nmPitch = normalMap->getPitch();
@ -721,12 +719,6 @@ Image* LoadPNGImage(const string& filename)
}
img = new Image(glformat, width, height);
if (img == nullptr)
{
fclose(fp);
png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp) nullptr);
return nullptr;
}
// TODO: consider using paletted textures if they're available
if (color_type == PNG_COLOR_TYPE_PALETTE)
@ -870,12 +862,6 @@ static Image* LoadBMPImage(ifstream& in)
// check for truncated file
auto* img = new Image(GL_RGB, imageHeader.width, imageHeader.height);
if (!img)
{
delete[] pixels;
delete[] palette;
return nullptr;
}
// Copy the image and perform any necessary conversions
for (int y = 0; y < imageHeader.height; y++)
@ -920,6 +906,7 @@ static Image* LoadBMPImage(ifstream& in)
}
delete[] pixels;
delete palette;
return img;
}

View File

@ -299,12 +299,9 @@ Model* LoadCelestiaMesh(const string& filename)
(int) params.rings, (int) params.slices,
NoiseDisplacementFunc,
(void*) &params);
if (sphereMesh != nullptr)
{
Mesh* mesh = sphereMesh->convertToMesh();
model->addMesh(mesh);
delete sphereMesh;
}
Mesh* mesh = sphereMesh->convertToMesh();
model->addMesh(mesh);
delete sphereMesh;
return model;
}
@ -514,16 +511,11 @@ ConvertTriangleMesh(M3DTriangleMesh& mesh,
delete[] faceNormals;
delete[] vertexNormals;
delete[] faceCounts;
if (vertexFaces != nullptr)
for (int i = 0; i < nVertices; i++)
{
for (int i = 0; i < nVertices; i++)
{
if (vertexFaces[i] != nullptr)
delete[] vertexFaces[i];
}
delete[] vertexFaces;
delete[] vertexFaces[i];
}
delete[] vertexFaces;
return newMesh;
}

View File

@ -122,7 +122,7 @@ ModelGeometry::render(RenderContext& rc, double /* t */)
Mesh* mesh = m_model->getMesh(meshIndex);
GLuint vboId = 0;
if (m_glData && meshIndex < m_glData->vbos.size())
if (meshIndex < m_glData->vbos.size())
{
vboId = m_glData->vbos[meshIndex];
}

View File

@ -111,13 +111,15 @@ uint32_t NameDatabase<OBJ>::getCatalogNumberByName(const std::string& name) cons
template <class OBJ>
std::string NameDatabase<OBJ>::getNameByCatalogNumber(const uint32_t catalogNumber) const
{
if (catalogNumber == OBJ::InvalidCatalogNumber)
if (catalogNumber == OBJ::InvalidCatalogNumber)
return "";
NumberIndex::const_iterator iter = numberIndex.lower_bound(catalogNumber);
NumberIndex::const_iterator iter = numberIndex.lower_bound(catalogNumber);
if (iter != numberIndex.end() && iter->first == catalogNumber)
return iter->second;
if (iter != numberIndex.end() && iter->first == catalogNumber)
return iter->second;
return "";
}

View File

@ -853,12 +853,9 @@ void Observer::convertFrameCoordinates(const ObserverFrame* newFrame)
void Observer::setFrame(ObserverFrame::CoordinateSystem cs, const Selection& refObj, const Selection& targetObj)
{
ObserverFrame* newFrame = new ObserverFrame(cs, refObj, targetObj);
if (newFrame != nullptr)
{
convertFrameCoordinates(newFrame);
delete frame;
frame = newFrame;
}
convertFrameCoordinates(newFrame);
delete frame;
frame = newFrame;
}

View File

@ -655,13 +655,10 @@ CreateScriptedOrbit(Hash* orbitData,
orbitData->addValue("AddonPath", *pathValue);
ScriptedOrbit* scriptedOrbit = new ScriptedOrbit();
if (scriptedOrbit != nullptr)
if (!scriptedOrbit->initialize(moduleName, funcName, orbitData))
{
if (!scriptedOrbit->initialize(moduleName, funcName, orbitData))
{
delete scriptedOrbit;
scriptedOrbit = nullptr;
}
delete scriptedOrbit;
scriptedOrbit = nullptr;
}
return scriptedOrbit;
@ -1030,13 +1027,10 @@ CreateScriptedRotation(Hash* rotationData,
rotationData->addValue("AddonPath", *pathValue);
ScriptedRotation* scriptedRotation = new ScriptedRotation();
if (scriptedRotation != nullptr)
if (!scriptedRotation->initialize(moduleName, funcName, rotationData))
{
if (!scriptedRotation->initialize(moduleName, funcName, rotationData))
{
delete scriptedRotation;
scriptedRotation = nullptr;
}
delete scriptedRotation;
scriptedRotation = nullptr;
}
return scriptedRotation;

View File

@ -5770,7 +5770,7 @@ void Renderer::renderCometTail(const Body& body,
}
else
{
v0 = v1 = cometPoints[i] - cometPoints[i - 1];
v0 = cometPoints[i] - cometPoints[i - 1];
sectionLength = v0.norm();
v0.normalize();
v1 = v0;

View File

@ -575,8 +575,6 @@ bool StarDatabase::loadCrossIndex(const Catalog catalog, istream& in)
}
CrossIndex* xindex = new CrossIndex();
if (xindex == nullptr)
return false;
unsigned int record = 0;
for (;;)

View File

@ -547,8 +547,6 @@ TiledTexture::TiledTexture(Image& img,
Image* tile = new Image(img.getFormat(),
tileWidth, tileHeight,
tileMipLevelCount);
if (tile == nullptr)
return;
for (int v = 0; v < vSplit; v++)
{
@ -824,8 +822,6 @@ Texture* CreateProceduralTexture(int width, int height,
Texture::MipMapMode mipMode)
{
Image* img = new Image(format, width, height);
if (img == nullptr)
return nullptr;
for (int y = 0; y < height; y++)
{
@ -851,8 +847,6 @@ Texture* CreateProceduralTexture(int width, int height,
Texture::MipMapMode mipMode)
{
Image* img = new Image(format, width, height);
if (img == nullptr)
return nullptr;
for (int y = 0; y < height; y++)
{
@ -911,32 +905,21 @@ extern Texture* CreateProceduralCubeMap(int size, int format,
ProceduralTexEval func)
{
Image* faces[6];
bool failed = false;
int i = 0;
for (i = 0; i < 6; i++)
for (int i = 0; i < 6; i++)
{
faces[i] = nullptr;
faces[i] = new Image(format, size, size);
if (faces[i] == nullptr)
failed = true;
}
if (!failed)
{
for (int i = 0; i < 6; i++)
Image* face = faces[i];
for (int y = 0; y < size; y++)
{
Image* face = faces[i];
for (int y = 0; y < size; y++)
for (int x = 0; x < size; x++)
{
for (int x = 0; x < size; x++)
{
float s = ((float) x + 0.5f) / (float) size * 2 - 1;
float t = ((float) y + 0.5f) / (float) size * 2 - 1;
Vector3f v = cubeVector(i, s, t);
func(v.x(), v.y(), v.z(),
face->getPixelRow(y) + x * face->getComponents());
}
float s = ((float) x + 0.5f) / (float) size * 2 - 1;
float t = ((float) y + 0.5f) / (float) size * 2 - 1;
Vector3f v = cubeVector(i, s, t);
func(v.x(), v.y(), v.z(),
face->getPixelRow(y) + x * face->getComponents());
}
}
}
@ -944,10 +927,9 @@ extern Texture* CreateProceduralCubeMap(int size, int format,
Texture* tex = new CubeMap(faces);
// Clean up the images
for (i = 0; i < 6; i++)
for (int i = 0; i < 6; i++)
{
if (faces[i] != nullptr)
delete faces[i];
delete faces[i];
}
return tex;

View File

@ -204,7 +204,7 @@ static void Nutation(double t, double &deps, double& dpsi)
deps = degToRad(deps/3600);
}
static void EclipticToEquatorial(double t, double fEclLat, double fEclLon,
static void EclipticToEquatorial(double fEclLat, double fEclLon,
double& RA, double& dec)
{
// Parameter t represents the Julian centuries elapsed since 1900.
@ -214,6 +214,7 @@ static void EclipticToEquatorial(double t, double fEclLat, double fEclLon,
double sx, cx, sy, cy, ty;
double eps;
double deps, dpsi;
double t;
// t = (astro::J2000 - 2415020.0) / 36525.0;
t = 0;
@ -721,7 +722,7 @@ class LunarOrbit : public CachingOrbit
#if 1
// Finally convert eclLat, eclLon to RA, Dec.
EclipticToEquatorial(t, eclLat, eclLon, RA, dec);
EclipticToEquatorial(eclLat, eclLon, RA, dec);
// RA and Dec are referred to the equinox of date; we want to use
// the J2000 equinox instead. A better idea would be to directly

View File

@ -170,8 +170,6 @@ JPLEphemeris* JPLEphemeris::load(istream& in)
return nullptr;
eph = new JPLEphemeris();
if (eph == nullptr)
return nullptr;
// Read the start time, end time, and time interval
eph->startDate = readDouble(in);

View File

@ -4280,9 +4280,6 @@ bool CelestiaCore::initRenderer()
Renderer::ShowAutoMag);
GLContext* context = new GLContext();
assert(context != nullptr);
if (context == nullptr)
return false;
context->init(config->ignoreGLExtensions);
// Choose the render path, starting with the least desirable

View File

@ -380,7 +380,7 @@ static int observer_goto(lua_State* l)
double startInter = celx.safeGetNumber(4, WrongType, "Third arg to observer:goto must be a number", 0.25);
double endInter = celx.safeGetNumber(5, WrongType, "Fourth arg to observer:goto must be a number", 0.75);
if (startInter < 0 || startInter > 1) startInter = 0.25;
if (endInter < 0 || endInter > 1) startInter = 0.75;
if (endInter < 0 || endInter > 1) endInter = 0.75;
// The first argument may be either an object or a position
if (sel != nullptr)

View File

@ -334,21 +334,22 @@ static int position_addvector(lua_State* l)
celx.checkArgs(2, 2, "One argument expected to position:addvector()");
UniversalCoord* uc = this_position(l);
if (uc == nullptr)
return 0;
auto v3d = celx.toVector(2);
if (v3d == nullptr)
{
celx.doError("Vector expected as argument to position:addvector");
return 0;
}
else
if (uc != nullptr && v3d != nullptr)
{
#ifdef __CELVEC__
UniversalCoord ucnew = uc->offsetUly(toEigen(*v3d));
UniversalCoord ucnew = uc->offsetUly(toEigen(*v3d));
#else
UniversalCoord ucnew = uc->offsetUly(*v3d);
UniversalCoord ucnew = uc->offsetUly(*v3d);
#endif
position_new(l, ucnew);
}
position_new(l, ucnew);
return 1;
}

View File

@ -116,7 +116,7 @@ double EclipseFinder::findEclipseSpan(const Body& receiver, const Body& caster,
}
int EclipseFinder::CalculateEclipses()
int EclipseFinder::CalculateEclipses() // XXX: this function is very fragile and should be rewritten
{
Simulation* sim = appCore->getSimulation();

View File

@ -445,7 +445,7 @@ BookmarkTreeModel::dropMimeData(const QMimeData* data, Qt::DropAction action, in
QByteArray encodedData = data->data("application/celestia.text.list");
QDataStream stream(&encodedData, QIODevice::ReadOnly);
BookmarkItem* item = nullptr;
BookmarkItem item;
// Read the pointer (ugh) from the encoded mime data. Bail out now
// if the data was incomplete for some reason.
@ -470,7 +470,7 @@ BookmarkTreeModel::dropMimeData(const QMimeData* data, Qt::DropAction action, in
// is inserted in the new position, then the old item is removed. Bad things
// happen when the item appears in two separate places, so we'll insert a copy
// of the old data into the new position.
BookmarkItem* clone = item->clone(parentFolder);
BookmarkItem* clone = item.clone(parentFolder);
beginInsertRows(parent, row, row);
parentFolder->insert(clone, row);
@ -517,7 +517,7 @@ BookmarkTreeModel::mimeData(const QModelIndexList& indexes) const
const BookmarkItem* item = getItem(indexes.at(0));
QDataStream stream(&encodedData, QIODevice::WriteOnly);
stream.writeRawData(reinterpret_cast<const char*>(&item), sizeof(item));
stream.writeRawData(reinterpret_cast<const char*>(&item), sizeof(*item));
mimeData->setData("application/celestia.text.list", encodedData);
return mimeData;
@ -666,6 +666,9 @@ BookmarkManager::appendBookmarkMenuItems(QMenu* menu, const BookmarkItem* item)
case BookmarkItem::Separator:
menu->addSeparator();
break;
default:
break;
}
}
}
@ -761,6 +764,9 @@ BookmarkToolBar::rebuild()
case BookmarkItem::Separator:
addSeparator();
break;
default:
break;
}
}
}

View File

@ -35,11 +35,14 @@ public:
{
Bookmark,
Folder,
Separator
Separator,
None
};
static const int ICON_SIZE = 24;
BookmarkItem() : m_type(None), m_parent(nullptr) {};
BookmarkItem(Type type, BookmarkItem* parent);
BookmarkItem::Type type() const;

View File

@ -88,8 +88,6 @@ std::vector<ScriptMenuItem>*
ScanScriptsDirectory(string scriptsDir, bool deep)
{
vector<ScriptMenuItem>* scripts = new vector<ScriptMenuItem>;
if (scripts == nullptr)
return nullptr;
Directory* dir = OpenDirectory(scriptsDir);

View File

@ -65,6 +65,9 @@ Mesh::VertexDescription::VertexDescription(const VertexDescription& desc) :
Mesh::VertexDescription&
Mesh::VertexDescription::operator=(const Mesh::VertexDescription& desc)
{
if (this == &desc)
return *this;
if (nAttributes < desc.nAttributes)
{
delete[] attributes;

View File

@ -1061,11 +1061,6 @@ AsciiModelLoader::loadVertices(const Mesh::VertexDescription& vertexDesc,
vertexCount = (unsigned int) num;
unsigned int vertexDataSize = vertexDesc.stride * vertexCount;
auto* vertexData = new char[vertexDataSize];
if (vertexData == nullptr)
{
reportError("Not enough memory to hold vertex data");
return nullptr;
}
unsigned int offset = 0;
double data[4];
@ -1197,12 +1192,6 @@ AsciiModelLoader::loadMesh()
unsigned int indexCount = (unsigned int) tok.currentToken().integerValue();
auto* indices = new Mesh::index32[indexCount];
if (indices == nullptr)
{
reportError("Not enough memory to hold indices");
delete mesh;
return nullptr;
}
for (unsigned int i = 0; i < indexCount; i++)
{
@ -1239,13 +1228,6 @@ AsciiModelLoader::load()
auto* model = new Model();
bool seenMeshes = false;
// FIXME: modern C++ uses exceptions
if (model == nullptr)
{
reportError("Unable to allocate memory for model");
return nullptr;
}
// Parse material and mesh definitions
for (Token token = tok.nextToken(); token.type() != Token::End; token = tok.nextToken())
{
@ -1769,12 +1751,6 @@ BinaryModelLoader::load()
auto* model = new Model();
bool seenMeshes = false;
if (model == nullptr)
{
reportError("Unable to allocate memory for model");
return nullptr;
}
// Parse material and mesh definitions
for (;;)
{
@ -2068,12 +2044,6 @@ BinaryModelLoader::loadMesh()
unsigned int indexCount = readUint(in);
auto* indices = new uint32_t[indexCount];
if (indices == nullptr)
{
reportError("Not enough memory to hold indices");
delete mesh;
return nullptr;
}
for (unsigned int i = 0; i < indexCount; i++)
{
@ -2109,11 +2079,6 @@ BinaryModelLoader::loadVertices(const Mesh::VertexDescription& vertexDesc,
vertexCount = readUint(in);
unsigned int vertexDataSize = vertexDesc.stride * vertexCount;
auto* vertexData = new char[vertexDataSize];
if (vertexData == nullptr)
{
reportError("Not enough memory to hold vertex data");
return nullptr;
}
unsigned int offset = 0;

View File

@ -415,12 +415,6 @@ TextureFont* TextureFont::load(istream& in)
if (format == TxfByte)
{
auto* fontImage = new unsigned char[texWidth * texHeight];
if (fontImage == nullptr)
{
DPRINTF(0, "Not enough memory for font bitmap.\n");
delete font;
return nullptr;
}
DPRINTF(1, "Reading %d x %d 8-bit font image.\n", texWidth, texHeight);
@ -440,16 +434,6 @@ TextureFont* TextureFont::load(istream& in)
int rowBytes = (texWidth + 7) >> 3;
auto* fontBits = new unsigned char[rowBytes * texHeight];
auto* fontImage = new unsigned char[texWidth * texHeight];
if (fontImage == nullptr || fontBits == nullptr)
{
DPRINTF(0, "Not enough memory for font bitmap.\n");
delete font;
delete[] fontBits;
delete[] fontImage;
return nullptr;
}
DPRINTF(1, "Reading %d x %d 1-bit font image.\n", texWidth, texHeight);
@ -458,6 +442,7 @@ TextureFont* TextureFont::load(istream& in)
{
DPRINTF(0, "Missing bitmap data in font stream.\n");
delete font;
delete[] fontImage;
return nullptr;
}