Fix the most serious fixes found by coverity

… resource leaks, null pointers dereference
pull/110/head
Hleb Valoshka 2018-05-10 01:09:09 +03:00
parent 0d002e490e
commit 5a5b06481f
32 changed files with 146 additions and 39 deletions

View File

@ -135,6 +135,7 @@ AsterismList* ReadAsterismList(istream& in, const StarDatabase& stardb)
{
DPRINTF(0, "Error parsing asterism %s\n", name.c_str());
for_each(asterisms->begin(), asterisms->end(), deleteFunc<Asterism*>());
delete ast;
delete asterisms;
delete chainsValue;
return NULL;

View File

@ -991,7 +991,7 @@ void RepeatCommand::process(ExecutionEnvironment& env, double t, double dt)
}
}
double RepeatCommand::getDuration()
double RepeatCommand::getDuration() const
{
return bodyDuration * repeatCount;
}

View File

@ -677,7 +677,7 @@ class RepeatCommand : public Command
RepeatCommand(CommandSequence* _body, int _repeatCount);
~RepeatCommand();
void process(ExecutionEnvironment&, double t, double dt) = 0;
double getDuration();
double getDuration() const;
private:
CommandSequence* body;

View File

@ -530,6 +530,7 @@ GalacticForm* buildGalacticForms(const std::string& filename)
if (img == NULL)
{
cout<<"\nThe galaxy template *** "<<filename<<" *** could not be loaded!\n\n";
delete galacticPoints;
return NULL;
}
width = img->getWidth();

View File

@ -196,6 +196,7 @@ GLShaderLoader::CreateVertexShader(const vector<string>& source,
*g_shaderLogFile << "Error compiling vertex shader:\n";
*g_shaderLogFile << GetInfoLog(shader->getID());
}
delete shader;
return status;
}
@ -223,6 +224,7 @@ GLShaderLoader::CreateFragmentShader(const vector<string>& source,
*g_shaderLogFile << "Error compiling fragment shader:\n";
*g_shaderLogFile << GetInfoLog(shader->getID());
}
delete shader;
return status;
}

View File

@ -629,7 +629,8 @@ Image* LoadJPEGImage(const string& filename, int)
void PNGReadData(png_structp png_ptr, png_bytep data, png_size_t length)
{
FILE* fp = (FILE*) png_get_io_ptr(png_ptr);
fread((void*) data, 1, length, fp);
if (fread((void*) data, 1, length, fp) != length)
cerr << "Error reading PNG data";
}
#endif
@ -655,8 +656,9 @@ Image* LoadPNGImage(const string& filename)
return NULL;
}
fread(header, 1, sizeof(header), fp);
if (png_sig_cmp((unsigned char*) header, 0, sizeof(header)))
size_t elements_read;
elements_read = fread(header, 1, sizeof(header), fp);
if (elements_read == 0 || png_sig_cmp((unsigned char*) header, 0, sizeof(header)))
{
clog << _("Error: ") << filename << _(" is not a PNG file.\n");
fclose(fp);
@ -874,6 +876,8 @@ static Image* LoadBMPImage(ifstream& in)
if (img == NULL)
{
delete[] pixels;
if (palette != NULL)
delete[] palette;
return NULL;
}

View File

@ -116,6 +116,8 @@ LODSphereMesh::~LODSphereMesh()
{
if (vertices != NULL)
delete[] vertices;
if (indices != NULL)
delete[] indices;
}

View File

@ -103,6 +103,10 @@ Observer::Observer(const Observer& o) :
updateUniversal();
}
Observer::~Observer()
{
delete frame;
}
Observer& Observer::operator=(const Observer& o)
{

View File

@ -108,6 +108,7 @@ public:
Observer();
Observer(const Observer& o);
~Observer();
Observer& operator=(const Observer& o);

View File

@ -779,7 +779,7 @@ GLSL_RenderContext::makeCurrent(const Material& m)
// a planet mesh. See SourceForge bug #1855894 for more details.
bool disableDepthWriteOnBlend = true;
if (shaderProps.hasScattering())
if (atmosphere != NULL && shaderProps.hasScattering())
{
prog->setAtmosphereParameters(*atmosphere, objRadius, objRadius);
disableDepthWriteOnBlend = false;

View File

@ -670,6 +670,8 @@ Renderer::~Renderer()
delete starVertexBuffer;
if (pointStarVertexBuffer != NULL)
delete pointStarVertexBuffer;
if (glareVertexBuffer != NULL)
delete glareVertexBuffer;
delete[] skyVertices;
delete[] skyIndices;
delete[] skyContour;

View File

@ -261,15 +261,18 @@ void renderEllipsoid_GLSL(const RenderInfo& ri,
}
}
if (shadprop.texUsage & ShaderProperties::CloudShadowTexture)
if (atmosphere != NULL)
{
prog->shadowTextureOffset = cloudTexOffset;
prog->cloudHeight = 1.0f + atmosphere->cloudHeight / radius;
}
if (shadprop.texUsage & ShaderProperties::CloudShadowTexture)
{
prog->shadowTextureOffset = cloudTexOffset;
prog->cloudHeight = 1.0f + atmosphere->cloudHeight / radius;
}
if (shadprop.hasScattering())
{
prog->setAtmosphereParameters(*atmosphere, radius, radius);
if (shadprop.hasScattering())
{
prog->setAtmosphereParameters(*atmosphere, radius, radius);
}
}
if (shadprop.hasEclipseShadows() != 0)
@ -487,11 +490,14 @@ void renderClouds_GLSL(const RenderInfo& ri,
ri.ambientColor.blue());
prog->textureOffset = texOffset;
float cloudRadius = radius + atmosphere->cloudHeight;
if (shadprop.hasScattering())
if (atmosphere != NULL)
{
prog->setAtmosphereParameters(*atmosphere, radius, cloudRadius);
float cloudRadius = radius + atmosphere->cloudHeight;
if (shadprop.hasScattering())
{
prog->setAtmosphereParameters(*atmosphere, radius, cloudRadius);
}
}
#if 0

View File

@ -3380,7 +3380,7 @@ CelestiaGLProgram::setEclipseShadowParameters(const LightingState& ls,
li < min(ls.nLights, MaxShaderLights);
li++)
{
if (shadows != NULL)
if (ls.shadows[li] != NULL)
{
unsigned int nShadows = min((size_t) MaxShaderEclipseShadows, ls.shadows[li]->size());

View File

@ -1264,11 +1264,11 @@ bool LoadSolarSystemObjects(istream& in,
}
else if (itemType == "AltSurface")
{
Surface* surface = new Surface();
Surface* surface = new Surface(); // FIXME: check
surface->color = Color(1.0f, 1.0f, 1.0f);
surface->hazeColor = Color(0.0f, 0.0f, 0.0f, 0.0f);
FillinSurface(objectData, surface, directory);
if (surface != NULL && parent.body() != NULL)
if (parent.body() != NULL)
parent.body()->addAlternateSurface(primaryName, surface);
else
sscError(tokenizer, _("bad alternate surface"));
@ -1311,6 +1311,12 @@ SolarSystem::SolarSystem(Star* _star) :
frameTree = new FrameTree(star);
}
SolarSystem::~SolarSystem()
{
delete planets;
delete frameTree;
}
Star* SolarSystem::getStar() const
{

View File

@ -22,6 +22,7 @@ class SolarSystem
{
public:
SolarSystem(Star*);
~SolarSystem();
Star* getStar() const;
Eigen::Vector3f getCenter() const;

View File

@ -68,6 +68,8 @@ SphereMesh::~SphereMesh()
delete[] texCoords;
if (indices != NULL)
delete[] indices;
if (tangents != NULL)
delete[] tangents;
}

View File

@ -578,6 +578,7 @@ bool StarDatabase::loadCrossIndex(const Catalog catalog, istream& in)
if (strncmp(header, CROSSINDEX_FILE_HEADER, headerLength))
{
cerr << _("Bad header for cross index\n");
delete[] header;
return false;
}
delete[] header;
@ -639,8 +640,10 @@ bool StarDatabase::loadBinary(istream& in)
int headerLength = strlen(FILE_HEADER);
char* header = new char[headerLength];
in.read(header, headerLength);
if (strncmp(header, FILE_HEADER, headerLength))
if (strncmp(header, FILE_HEADER, headerLength)) {
delete[] header;
return false;
}
delete[] header;
}
@ -896,8 +899,12 @@ bool StarDatabase::createStar(Star* star,
// If the star definition has extended information, clone the
// star details so we can customize it without affecting other
// stars of the same spectral type.
bool free_details = false;
if (!modifyExistingDetails)
{
details = new StarDetails(*details);
free_details = true;
}
if (hasTexture)
{
@ -997,6 +1004,9 @@ bool StarDatabase::createStar(Star* star,
if (!hasBarycenter)
{
cerr << _("Barycenter ") << barycenterName << _(" does not exist.\n");
delete rm;
if (free_details)
delete details;
return false;
}
}

View File

@ -950,7 +950,7 @@ extern Texture* CreateProceduralCubeMap(int size, int format,
{
faces[i] = NULL;
faces[i] = new Image(format, size, size);
if (faces == NULL)
if (faces[i] == NULL)
failed = true;
}

View File

@ -76,7 +76,10 @@ int TimelinePhase::release() const
--refCount;
assert(refCount >= 0);
if (refCount <= 0)
{
delete this;
return 0;
}
return refCount;
}

View File

@ -215,7 +215,7 @@ static void EclipticToEquatorial(double t, double fEclLat, double fEclLon,
double eps;
double deps, dpsi;
t = (astro::J2000 - 2415020.0) / 36525.0;
// t = (astro::J2000 - 2415020.0) / 36525.0;
t = 0;
eps = Obliquity(t); // mean obliquity for date
Nutation(t, deps, dpsi);

View File

@ -409,6 +409,8 @@ CelestiaCore::~CelestiaCore()
#endif
delete execEnv;
delete timer;
delete renderer;
}
@ -3451,6 +3453,7 @@ void CelestiaCore::renderOverlay()
int fontHeight = font->getHeight();
int emWidth = font->getWidth("M");
assert(emWidth > 0);
overlay->begin();
@ -3549,13 +3552,9 @@ void CelestiaCore::renderOverlay()
lt = v.norm() / (86400.0 * astro::speedOfLight);
}
}
else
{
lt = 0.0;
}
double tdb = sim->getTime() + lt;
astro::Date d = timeZoneBias != 0?astro::TDBtoLocal(tdb):astro::TDBtoUTC(tdb);
astro::Date d = timeZoneBias != 0 ? astro::TDBtoLocal(tdb) : astro::TDBtoUTC(tdb);
const char* dateStr = d.toCStr(dateFormat);
int dateWidth = (font->getWidth(dateStr)/(emWidth * 3) + 2) * emWidth * 3;
if (dateWidth > dateStrWidth) dateStrWidth = dateWidth;
@ -4584,13 +4583,15 @@ bool CelestiaCore::readStars(const CelestiaConfig& cfg,
{
cerr << _("Error opening ") << cfg.starDatabaseFile << '\n';
delete starDB;
delete starNameDB;
return false;
}
if (!starDB->loadBinary(starFile))
{
delete starDB;
cerr << _("Error reading stars file\n");
delete starDB;
delete starNameDB;
return false;
}
}
@ -5208,7 +5209,10 @@ bool CelestiaCore::initLuaHook(ProgressNotifier* progressNotifier)
// create a private context.
if (config->scriptSystemAccessPolicy == "allow")
{
SetScriptedObjectContext(luaHook->getState());
if (luaHook)
{
SetScriptedObjectContext(luaHook->getState());
}
}
else
{
@ -5225,6 +5229,7 @@ bool CelestiaCore::initLuaHook(ProgressNotifier* progressNotifier)
{
delete luaSandbox;
luaSandbox = NULL;
return false;
}
SetScriptedObjectContext(luaSandbox->getState());

View File

@ -563,6 +563,7 @@ static void checkTimeslice(lua_State* l, lua_Debug* /*ar*/)
{
lua_pushstring(l, "Internal Error: Invalid value in checkTimeslice");
lua_error(l);
return;
}
if (luastate->timesliceExpired())
@ -1439,6 +1440,7 @@ static CelScriptWrapper* this_celscript(lua_State* l)
if (script == NULL)
{
Celx_DoError(l, "Bad CEL-script object!");
return NULL;
}
return *script;
}
@ -2050,6 +2052,13 @@ static int celestia_setconstellationcolor(lua_State* l)
if (lua_isstring(l, -1))
{
constellation = lua_tostring(l, -1);
for (AsterismList::const_iterator iter = asterisms->begin();
iter != asterisms->end(); iter++)
{
Asterism* ast = *iter;
if (compareIgnoringCase(constellation, ast->getName(false)) == 0)
ast->setOverrideColor(constellationColor);
}
}
else
{
@ -3163,6 +3172,7 @@ static int celestia_newrotation(lua_State* l)
if (v == NULL)
{
Celx_DoError(l, "newrotation: first argument must be a vector");
return 0;
}
double angle = Celx_SafeGetNumber(l, 3, AllErrors, "second argument to celestia:newrotation must be a number");
Quatd q;
@ -3211,6 +3221,7 @@ static int celestia_newframe(lua_State* l)
if (ref == NULL || target == NULL)
{
Celx_DoError(l, "newframe: two objects required for lock frame");
return 0;
}
frame_new(l, ObserverFrame(coordSys, *ref, *target));
@ -3222,6 +3233,7 @@ static int celestia_newframe(lua_State* l)
if (ref == NULL)
{
Celx_DoError(l, "newframe: one object argument required for frame");
return 0;
}
frame_new(l, ObserverFrame(coordSys, *ref));

View File

@ -80,6 +80,7 @@ static int frame_from(lua_State* l)
if (uc == NULL && q == NULL)
{
celx.doError("Position or rotation expected as second argument to frame:from()");
return 0;
}
jd = celx.safeGetNumber(3, WrongType, "Second arg to frame:from must be a number", appCore->getSimulation()->getTime());
@ -124,6 +125,7 @@ static int frame_to(lua_State* l)
if (uc == NULL && q == NULL)
{
celx.doError("Position or rotation expected as second argument to frame:to()");
return 0;
}
jd = celx.safeGetNumber(3, WrongType, "Second arg to frame:to must be a number", appCore->getSimulation()->getTime());

View File

@ -90,6 +90,7 @@ static int observer_setposition(lua_State* l)
if (uc == NULL)
{
celx.doError("Argument to observer:setposition must be a position");
return 0;
}
o->setPosition(*uc);
return 0;
@ -106,6 +107,7 @@ static int observer_setorientation(lua_State* l)
if (q == NULL)
{
celx.doError("Argument to observer:setorientation must be a rotation");
return 0;
}
o->setOrientation(toEigen(*q));
return 0;
@ -133,6 +135,7 @@ static int observer_rotate(lua_State* l)
if (q == NULL)
{
celx.doError("Argument to observer:setpos must be a rotation");
return 0;
}
Quatf qf((float) q->w, (float) q->x, (float) q->y, (float) q->z);
o->rotate(toEigen(qf));
@ -150,6 +153,7 @@ static int observer_orbit(lua_State* l)
if (q == NULL)
{
celx.doError("Argument for observer:orbit must be a rotation");
return 0;
}
Quatf qf((float) q->w, (float) q->x, (float) q->y, (float) q->z);
o->orbit(Selection(), toEigen(qf));
@ -174,6 +178,7 @@ static int observer_lookat(lua_State* l)
if (to == NULL)
{
celx.doError("Argument 1 (of 2) to observer:lookat must be of type position");
return 0;
}
}
else
@ -187,6 +192,7 @@ static int observer_lookat(lua_State* l)
if (to == NULL || from == NULL)
{
celx.doError("Argument 1 and 2 (of 3) to observer:lookat must be of type position");
return 0;
}
}
}
@ -194,6 +200,7 @@ static int observer_lookat(lua_State* l)
if (upd == NULL)
{
celx.doError("Last argument to observer:lookat must be of type vector");
return 0;
}
Vector3d nd;
if (from == NULL)
@ -320,6 +327,7 @@ static int observer_goto(lua_State* l)
if (sel == NULL && uc == NULL)
{
celx.doError("First arg to observer:goto must be object or position");
return 0;
}
double travelTime = celx.safeGetNumber(3, WrongType, "Second arg to observer:goto must be a number", 5.0);
@ -352,6 +360,7 @@ static int observer_gotolonglat(lua_State* l)
if (sel == NULL)
{
celx.doError("First arg to observer:gotolonglat must be an object");
return 0;
}
double defaultDistance = sel->radius() * 5.0;
@ -369,6 +378,7 @@ static int observer_gotolonglat(lua_State* l)
if (uparg == NULL)
{
celx.doError("Sixth argument to observer:gotolonglat must be a vector");
return 0;
}
up = toEigen(*uparg).cast<float>();
}
@ -412,6 +422,7 @@ static int observer_gotodistance(lua_State* l)
if (sel == NULL)
{
celx.doError("First arg to observer:gotodistance must be object");
return 0;
}
double distance = celx.safeGetNumber(3, WrongType, "Second arg to observer:gotodistance must be a number", 20000);
@ -424,6 +435,7 @@ static int observer_gotodistance(lua_State* l)
if (up_arg == NULL)
{
celx.doError("Fourth arg to observer:gotodistance must be a vector");
return 0;
}
up = toEigen(*up_arg).cast<float>();
@ -444,6 +456,7 @@ static int observer_gotosurface(lua_State* l)
if (sel == NULL)
{
celx.doError("First arg to observer:gotosurface must be object");
return 0;
}
double travelTime = celx.safeGetNumber(3, WrongType, "Second arg to observer:gotosurface must be a number", 5.0);
@ -465,6 +478,7 @@ static int observer_center(lua_State* l)
if (sel == NULL)
{
celx.doError("First argument to observer:center must be an object");
return 0;
}
double travelTime = celx.safeGetNumber(3, WrongType, "Second arg to observer:center must be a number", 5.0);
@ -483,6 +497,7 @@ static int observer_centerorbit(lua_State* l)
if (sel == NULL)
{
celx.doError("First argument to observer:centerorbit must be an object");
return 0;
}
double travelTime = celx.safeGetNumber(3, WrongType, "Second arg to observer:centerorbit must be a number", 5.0);
@ -512,6 +527,7 @@ static int observer_follow(lua_State* l)
if (sel == NULL)
{
celx.doError("First argument to observer:follow must be an object");
return 0;
}
o->follow(*sel);
@ -528,6 +544,7 @@ static int observer_synchronous(lua_State* l)
if (sel == NULL)
{
celx.doError("First argument to observer:synchronous must be an object");
return 0;
}
o->geosynchronousFollow(*sel);
@ -544,6 +561,7 @@ static int observer_lock(lua_State* l)
if (sel == NULL)
{
celx.doError("First argument to observer:phaseLock must be an object");
return 0;
}
o->phaseLock(*sel);
@ -560,6 +578,7 @@ static int observer_chase(lua_State* l)
if (sel == NULL)
{
celx.doError("First argument to observer:chase must be an object");
return 0;
}
o->chase(*sel);
@ -585,6 +604,7 @@ static int observer_track(lua_State* l)
if (sel == NULL)
{
celx.doError("First argument to observer:center must be an object");
return 0;
}
o->setTrackedObject(*sel);
}

View File

@ -162,6 +162,7 @@ static int position_vectorto(lua_State* l)
if (uc2 == NULL)
{
celx.doError("Argument to position:vectorto must be a position");
return 0;
}
celx.newVector(fromEigen(uc2->offsetFromUly(*uc)));
@ -182,12 +183,14 @@ static int position_orientationto(lua_State* l)
if (target == NULL)
{
celx.doError("First argument to position:orientationto must be a position");
return 1;
}
Vec3d* upd = celx.toVector(3);
if (upd == NULL)
{
celx.doError("Second argument to position:orientationto must be a vector");
return 1;
}
Vec3d src2target = fromEigen(target->offsetFromKm(*src));
@ -222,6 +225,7 @@ static int position_distanceto(lua_State* l)
if (uc2 == NULL)
{
celx.doError("Position expected as argument to position:distanceto");
return 0;
}
lua_pushnumber(l, uc2->offsetFromKm(*uc).norm());

View File

@ -138,6 +138,7 @@ static int rotation_transform(lua_State* l)
if (v == NULL)
{
celx.doError("Argument to rotation:transform() must be a vector");
return 0;
}
vector_new(l, *v * q->toMatrix3());
return 1;
@ -154,6 +155,7 @@ static int rotation_setaxisangle(lua_State* l)
if (v == NULL)
{
celx.doError("setaxisangle: first argument must be a vector");
return 0;
}
double angle = celx.safeGetNumber(3, AllErrors, "second argument to rotation:setaxisangle must be a number");
q->setAxisAngle(*v, angle);
@ -171,6 +173,7 @@ static int rotation_slerp(lua_State* l)
if (q2 == NULL)
{
celx.doError("slerp: first argument must be a rotation");
return 0;
}
double t = celx.safeGetNumber(3, AllErrors, "second argument to rotation:slerp must be a number");
rotation_new(l, Quatd::slerp(*q1, *q2, t));

View File

@ -39,6 +39,10 @@ Eclipse::Eclipse(double JD) :
date = new astro::Date(JD);
}
Eclipse::~Eclipse()
{
delete date;
}
// TODO: share this constant and function with render.cpp
static const float MinRelativeOccluderRadius = 0.005f;
@ -216,7 +220,7 @@ int EclipseFinder::CalculateEclipses()
JDfrom += 1.0 / 24.0;
}
if (JDback)
delete JDback;
delete[] JDback;
if (Eclipses_.empty())
{
eclipse = new Eclipse(0.);

View File

@ -22,6 +22,7 @@ class Eclipse
public:
Eclipse(int Y, int M, int D);
Eclipse(double JD);
~Eclipse();
enum Type {
Solar = 0,

View File

@ -36,7 +36,7 @@ FavoritesList* ReadFavoritesList(istream& in)
return NULL;
}
FavoritesEntry* fav = new FavoritesEntry();
FavoritesEntry* fav = new FavoritesEntry(); // FIXME: check
fav->name = tokenizer.getStringValue();
Value* favParamsValue = parser.readValue();
@ -47,6 +47,7 @@ FavoritesList* ReadFavoritesList(istream& in)
delete favorites;
if (favParamsValue != NULL)
delete favParamsValue;
delete fav;
return NULL;
}

View File

@ -734,7 +734,8 @@ private:
};
ModelLoader::ModelLoader()
ModelLoader::ModelLoader() :
textureLoader(NULL)
{
}
@ -1291,6 +1292,7 @@ AsciiModelLoader::load()
Model* model = new Model();
bool seenMeshes = false;
// FIXME: modern C++ uses exceptions
if (model == NULL)
{
reportError("Unable to allocate memory for model");
@ -1346,6 +1348,7 @@ AsciiModelLoader::load()
else
{
reportError("Block name expected");
delete model;
return NULL;
}
}
@ -2142,7 +2145,7 @@ BinaryModelLoader::loadMesh()
if (index >= vertexCount)
{
reportError("Index out of range");
delete indices;
delete[] indices;
delete mesh;
return NULL;
}

View File

@ -14,6 +14,7 @@
#include <math.h>
#include <stdio.h>
#include <iostream>
#include "bigfix.h"
@ -86,10 +87,17 @@ BigFix::BigFix(double d)
hi = ((uint64) w3 << 32) | w2;
lo = ((uint64) w1 << 32) | w0;
}
if (isNegative)
negate128(hi, lo);
if (isNegative)
negate128(hi, lo);
}
else
{
// Not a good idea but at least they are initialized
// if too big (>= 2**63) value is passed
std::cerr << "Too big value " << d <<" passed to BigFix::BigFix()\n";
hi = lo = 0;
}
}

View File

@ -101,8 +101,7 @@ Directory* OpenDirectory(const std::string& dirname)
bool IsDirectory(const std::string& filename)
{
struct stat buf;
stat(filename.c_str(), &buf);
return S_ISDIR(buf.st_mode);
return stat(filename.c_str(), &buf) == 0 ? S_ISDIR(buf.st_mode) : false;
}
std::string WordExp(const std::string& filename)