Merge pull request #5645 from jherico/maggie

Fixing problems reported by static analysis tool

assignment-client/src/AssignmentActionFactory.cpp

@@ -13,10 +13,9 @@
 
 
 EntityActionPointer assignmentActionFactory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity) {
-    return (EntityActionPointer) new AssignmentAction(type, id, ownerEntity);
+    return EntityActionPointer(new AssignmentAction(type, id, ownerEntity));
 }
 
-
 EntityActionPointer AssignmentActionFactory::factory(EntityActionType type,
                                                      const QUuid& id,
                                                      EntityItemPointer ownerEntity,

assignment-client/src/octree/OctreeInboundPacketProcessor.cpp

@@ -131,9 +131,6 @@ void OctreeInboundPacketProcessor::processPacket(QSharedPointer<NLPacket> packet
             qDebug() << "    numBytesPacketHeader=" << packet->totalHeadersSize();
             qDebug() << "    sizeof(sequence)=" << sizeof(sequence);
             qDebug() << "    sizeof(sentAt)=" << sizeof(sentAt);
-        }
-
-        if (debugProcessPacket) {
             qDebug() << "    atByte (in payload)=" << packet->pos();
             qDebug() << "    payload size=" << packet->getPayloadSize();
 

assignment-client/src/octree/OctreeServer.cpp

@@ -643,7 +643,7 @@ bool OctreeServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url
         quint64 averageLoggingTime = _tree->getAverageLoggingTime();
 
 
-        float averageElementsPerPacket = totalPacketsProcessed == 0 ? 0 : totalElementsProcessed / totalPacketsProcessed;
+        float averageElementsPerPacket = totalPacketsProcessed == 0 ? 0 : (float)totalElementsProcessed / totalPacketsProcessed;
 
         statsString += QString("   Current Inbound Packets Queue: %1 packets\r\n")
             .arg(locale.toString((uint)currentPacketsInQueue).rightJustified(COLUMN_WIDTH, ' '));
@@ -695,7 +695,7 @@ bool OctreeServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url
             totalElementsProcessed = senderStats.getTotalElementsProcessed();
             totalPacketsProcessed = senderStats.getTotalPacketsProcessed();
 
-            averageElementsPerPacket = totalPacketsProcessed == 0 ? 0 : totalElementsProcessed / totalPacketsProcessed;
+            averageElementsPerPacket = totalPacketsProcessed == 0 ? 0 : (float)totalElementsProcessed / totalPacketsProcessed;
 
             statsString += QString("               Total Inbound Packets: %1 packets\r\n")
                 .arg(locale.toString((uint)totalPacketsProcessed).rightJustified(COLUMN_WIDTH, ' '));
@@ -1075,9 +1075,7 @@ void OctreeServer::run() {
         // now set up PersistThread
         _persistThread = new OctreePersistThread(_tree, _persistFilename, _persistInterval,
                                                  _wantBackup, _settings, _debugTimestampNow, _persistAsFileType);
-        if (_persistThread) {
+        _persistThread->initialize(true);
-            _persistThread->initialize(true);
-        }
     }
 
     HifiSockAddr senderSockAddr;

interface/src/Application.cpp

@@ -1427,7 +1427,6 @@ void Application::keyPressEvent(QKeyEvent* event) {
         bool isMeta = event->modifiers().testFlag(Qt::ControlModifier);
         bool isOption = event->modifiers().testFlag(Qt::AltModifier);
         switch (event->key()) {
-                break;
             case Qt::Key_Enter:
             case Qt::Key_Return:
                 if (isOption) {

interface/src/GLCanvas.cpp

@@ -62,7 +62,7 @@ void GLCanvas::paintGL() {
     // FIXME - I'm not sure why this still remains, it appears as if this GLCanvas gets a single paintGL call near
     // the beginning of the application starting up. I'm not sure if we really need to call Application::paintGL()
     // in this case, since the display plugins eventually handle all the painting
-    if ((!Application::getInstance()->getWindow()->isMinimized() || !Application::getInstance()->isThrottleFPSEnabled())) {
+    if (!Application::getInstance()->getWindow()->isMinimized() || !Application::getInstance()->isThrottleFPSEnabled()) {
         Application::getInstance()->paintGL();
     }
 }

interface/src/ModelSelector.cpp

@@ -65,9 +65,8 @@ FSTReader::ModelType ModelSelector::getModelType() const {
         return FSTReader::ATTACHMENT_MODEL;
     } else if (text == ENTITY_MODEL_STRING) {
         return FSTReader::ENTITY_MODEL;
-    } else {
+    } 
-        Q_UNREACHABLE();
+    Q_UNREACHABLE();
-    }
 }
 
 void ModelSelector::accept() {

interface/src/avatar/MyAvatar.cpp

@@ -1307,7 +1307,7 @@ glm::vec3 MyAvatar::applyKeyboardMotor(float deltaTime, const glm::vec3& localVe
     bool isThrust = (glm::length2(_thrust) > EPSILON);
     if (_isPushing || isThrust ||
             (_scriptedMotorTimescale < MAX_KEYBOARD_MOTOR_TIMESCALE &&
-            _motionBehaviors | AVATAR_MOTION_SCRIPTED_MOTOR_ENABLED)) {
+            (_motionBehaviors & AVATAR_MOTION_SCRIPTED_MOTOR_ENABLED))) {
         // we don't want to brake if something is pushing the avatar around
         timescale = _keyboardMotorTimescale;
         _isBraking = false;

interface/src/ui/OctreeStatsDialog.cpp

@@ -437,7 +437,6 @@ void OctreeStatsDialog::showOctreeServersOfType(int& serverCount, NodeType_t ser
                         case MOST: {
                             extraDetails << "<br/>" ;
                             
-                            const unsigned long USECS_PER_MSEC = 1000;
                             float lastFullEncode = stats.getLastFullTotalEncodeTime() / USECS_PER_MSEC;
                             float lastFullSend = stats.getLastFullElapsedTime() / USECS_PER_MSEC;
                             float lastFullSendInSeconds = stats.getLastFullElapsedTime() / USECS_PER_SECOND;

libraries/animation/src/JointState.h

@@ -27,9 +27,10 @@ class AngularConstraint;
 class JointState {
 public:
     JointState() {}
-    JointState(const JointState& other) : _constraint(NULL) { copyState(other); }
+    JointState(const JointState& other) { copyState(other); }
     JointState(const FBXJoint& joint);
     ~JointState();
+    JointState& operator=(const JointState& other) { copyState(other); return *this; }
     void copyState(const JointState& state);
     void buildConstraint();
  

libraries/audio/src/AudioBuffer.h

@@ -77,10 +77,8 @@ AudioFrameBuffer< T >::~AudioFrameBuffer() {
 template< typename T >
 void AudioFrameBuffer< T >::allocateFrames() {
     _frameBuffer = new T*[_channelCountMax];
-    if (_frameBuffer) {
+    for (uint32_t i = 0; i < _channelCountMax; ++i) {
-        for (uint32_t i = 0; i < _channelCountMax; ++i) {
+        _frameBuffer[i] = new T[_frameCountMax];
-            _frameBuffer[i] = new T[_frameCountMax];
-        }
     }
 }
 

libraries/audio/src/AudioFilterBank.h

@@ -108,14 +108,14 @@ public:
 
     void setParameters(uint32_t filterStage, uint32_t filterChannel, const float32_t sampleRate, const float32_t frequency, 
                        const float32_t gain, const float32_t slope) {
-        if (filterStage >= 0 && filterStage < _filterCount && filterChannel >= 0 && filterChannel < _channelCount) {
+        if (filterStage < _filterCount && filterChannel >= 0 && filterChannel < _channelCount) {
             _filters[filterStage][filterChannel].setParameters(sampleRate,frequency,gain,slope);
         }
     }
 
     void getParameters(uint32_t filterStage, uint32_t filterChannel, float32_t& sampleRate, float32_t& frequency, 
                        float32_t& gain, float32_t& slope) {
-        if (filterStage >= 0 && filterStage < _filterCount && filterChannel >= 0 && filterChannel < _channelCount) {
+        if (filterStage < _filterCount && filterChannel >= 0 && filterChannel < _channelCount) {
             _filters[filterStage][filterChannel].getParameters(sampleRate,frequency,gain,slope);
         }
     }

libraries/audio/src/AudioRingBuffer.h

@@ -85,12 +85,12 @@ public:
             _bufferFirst(NULL),
             _bufferLast(NULL),
             _at(NULL) {}
-
         ConstIterator(int16_t* bufferFirst, int capacity, int16_t* at)
             : _bufferLength(capacity),
             _bufferFirst(bufferFirst),
             _bufferLast(bufferFirst + capacity - 1),
             _at(at) {}
+        ConstIterator(const ConstIterator& rhs) = default;
 
         bool isNull() const { return _at == NULL; }
 

libraries/fbx/src/FBXReader.cpp

@@ -2003,6 +2003,7 @@ FBXGeometry* extractFBXGeometry(const FBXNode& node, const QVariantHash& mapping
 
                     material._material = make_shared<model::Material>();
                     material._material->setEmissive(material.emissive);
+                    // FIXME both cases are identical
                     if (glm::all(glm::equal(material.diffuse, glm::vec3(0.0f)))) {
                         material._material->setDiffuse(material.diffuse); 
                     } else {

libraries/gpu/src/gpu/Framebuffer.h

@@ -149,7 +149,7 @@ protected:
     void updateSize(const TexturePointer& texture);
 
     // Non exposed
-    Framebuffer(const Framebuffer& framebuffer) {}
+    Framebuffer(const Framebuffer& framebuffer) = delete;
     Framebuffer() {}
     
     // This shouldn't be used by anything else than the Backend class with the proper casting.

libraries/input-plugins/src/input-plugins/ViveControllerManager.cpp

@@ -95,7 +95,7 @@ void ViveControllerManager::activate() {
 
     vr::RenderModel_t model;
     if (!_hmd->LoadRenderModel(CONTROLLER_MODEL_STRING.toStdString().c_str(), &model)) {
-        qDebug("Unable to load render model %s\n", CONTROLLER_MODEL_STRING);
+        qDebug() << QString("Unable to load render model %1\n").arg(CONTROLLER_MODEL_STRING);
     } else {
         model::Mesh* mesh = new model::Mesh();
         model::MeshPointer meshPtr(mesh);
@@ -198,7 +198,7 @@ void ViveControllerManager::renderHand(UserInputMapper::PoseValue pose, gpu::Bat
     Transform transform(userInputMapper->getSensorToWorldMat());
     transform.postTranslate(pose.getTranslation() + pose.getRotation() * glm::vec3(0, 0, CONTROLLER_LENGTH_OFFSET));
 
-    int sign = index == LEFT_HAND ? 1.0f : -1.0f;
+    int sign = index == LEFT_HAND ? 1 : -1;
     glm::quat rotation = pose.getRotation() * glm::angleAxis(PI, glm::vec3(1.0f, 0.0f, 0.0f)) * glm::angleAxis(sign * PI_OVER_TWO, glm::vec3(0.0f, 0.0f, 1.0f));
     transform.postRotate(rotation);
 
@@ -325,7 +325,7 @@ void ViveControllerManager::handlePoseEvent(const mat4& mat, int index) {
     glm::quat rotation = glm::quat_cast(mat);
 
     // Flip the rotation appropriately for each hand
-    int sign = index == LEFT_HAND ? 1.0f : -1.0f;
+    int sign = index == LEFT_HAND ? 1 : -1;
     rotation = rotation * glm::angleAxis(PI, glm::vec3(1.0f, 0.0f, 0.0f)) * glm::angleAxis(sign * PI_OVER_TWO, glm::vec3(0.0f, 0.0f, 1.0f));
 
     position += rotation * glm::vec3(0, 0, -CONTROLLER_LENGTH_OFFSET);

libraries/octree/src/CoverageMap.cpp

@@ -338,7 +338,7 @@ void CoverageRegion::erase() {
     }
 **/
     // If we're in charge of managing the polygons, then clean them up first
-    if (_managePolygons) {
+    if (_polygons && _managePolygons) {
         for (int i = 0; i < _polygonCount; i++) {
             delete _polygons[i];
             _polygons[i] = NULL; // do we need to do this?

libraries/octree/src/Octree.cpp

@@ -1318,7 +1318,8 @@ int Octree::encodeTreeBitstreamRecursion(OctreeElement* element,
                 // If the user also asked for occlusion culling, check if this element is occluded
                 if (params.wantOcclusionCulling && childElement->isLeaf()) {
                     // Don't check occlusion here, just add them to our distance ordered array...
-
+                    
+                    // FIXME params.ViewFrustum is used here, but later it is checked against nullptr.
                     OctreeProjectedPolygon* voxelPolygon = new OctreeProjectedPolygon(
                                 params.viewFrustum->getProjectedPolygon(childElement->getAACube()));
 

libraries/render-utils/src/TextureCache.cpp

@@ -621,6 +621,7 @@ QSharedPointer<Texture> DilatableNetworkTexture::getDilatedTexture(float dilatio
             gpu::Element formatMip = gpu::Element(gpu::VEC3, gpu::UINT8, (isLinearRGB ? gpu::RGB : gpu::SRGB));
             if (dilatedImage.hasAlphaChannel()) {
                 formatGPU = gpu::Element(gpu::VEC4, gpu::UINT8, (isLinearRGB ? gpu::RGBA : gpu::SRGBA));
+                // FIXME either remove the ?: operator or provide different arguments depending on linear
                 formatMip = gpu::Element(gpu::VEC4, gpu::UINT8, (isLinearRGB ? gpu::BGRA : gpu::BGRA));
             }
             texture->_gpuTexture = gpu::TexturePointer(gpu::Texture::create2D(formatGPU, dilatedImage.width(), dilatedImage.height(), gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_MIP_LINEAR)));

libraries/script-engine/src/WebSocketClass.h

@@ -81,8 +81,10 @@ public:
                 return OPEN;
             case QAbstractSocket::SocketState::ClosingState:
                 return CLOSING;
+            case QAbstractSocket::SocketState::UnconnectedState:
+            default:
+                return CLOSED;
         }
-        return CLOSED;
     }
 
     void setOnClose(QScriptValue eventFunction) { _onCloseEvent = eventFunction; }

libraries/shared/src/Extents.cpp

@@ -20,8 +20,8 @@
 #include "Transform.h"
 
 void Extents::reset() {
-    minimum = glm::vec3(FLT_MAX);
+    minimum = Vectors::MAX;
-    maximum = glm::vec3(-FLT_MAX);
+    maximum = Vectors::MIN;
 }
 
 bool Extents::containsPoint(const glm::vec3& point) const {

libraries/shared/src/Extents.h

@@ -18,14 +18,15 @@
 
 #include <QDebug>
 #include "StreamUtils.h"
+#include "GLMHelpers.h"
 
 class AABox;
 class Transform;
 
 class Extents {
 public:
-    Extents(const glm::vec3& minimum, const glm::vec3& maximum) : minimum(minimum), maximum(maximum)  { }
+    Extents() { }
-    Extents() { reset(); }
+    Extents(const glm::vec3& minimum, const glm::vec3& maximum) : minimum(minimum), maximum(maximum) {}
     Extents(const AABox& box) { reset(); add(box); }
 
     /// set minimum and maximum to FLT_MAX and -FLT_MAX respectively
@@ -49,7 +50,7 @@ public:
 
     /// \return whether or not the extents are empty
     bool isEmpty() const { return minimum == maximum; }
-    bool isValid() const { return !((minimum == glm::vec3(FLT_MAX)) && (maximum == glm::vec3(-FLT_MAX))); }
+    bool isValid() const { return !((minimum == Vectors::MAX) && (maximum == Vectors::MIN)); }
 
     /// \param vec3 for delta amount to shift the extents by
     /// \return true if point is within current limits
@@ -75,8 +76,8 @@ public:
         return temp;
     }
 
-    glm::vec3 minimum;
+    glm::vec3 minimum{ Vectors::MAX };
-    glm::vec3 maximum;
+    glm::vec3 maximum{ Vectors::MIN };
 };
 
 inline QDebug operator<<(QDebug debug, const Extents& extents) {

libraries/shared/src/GLMHelpers.cpp

@@ -13,6 +13,24 @@
 
 #include "NumericalConstants.h"
 
+const vec3 Vectors::UNIT_X{ 1.0f, 0.0f, 0.0f };
+const vec3 Vectors::UNIT_Y{ 0.0f, 1.0f, 0.0f };
+const vec3 Vectors::UNIT_Z{ 0.0f, 0.0f, 1.0f };
+const vec3 Vectors::UNIT_NEG_X{ -1.0f, 0.0f, 0.0f };
+const vec3 Vectors::UNIT_NEG_Y{ 0.0f, -1.0f, 0.0f };
+const vec3 Vectors::UNIT_NEG_Z{ 0.0f, 0.0f, -1.0f };
+const vec3 Vectors::UNIT_XY{ glm::normalize(UNIT_X + UNIT_Y) };
+const vec3 Vectors::UNIT_XZ{ glm::normalize(UNIT_X + UNIT_Z) };
+const vec3 Vectors::UNIT_YZ{ glm::normalize(UNIT_Y + UNIT_Z) };
+const vec3 Vectors::UNIT_XYZ{ glm::normalize(UNIT_X + UNIT_Y + UNIT_Z) };
+const vec3 Vectors::MAX{ FLT_MAX };
+const vec3 Vectors::MIN{ FLT_MIN };
+const vec3 Vectors::ZERO{ 0.0f };
+const vec3 Vectors::ONE{ 1.0f };
+const vec3& Vectors::RIGHT = Vectors::UNIT_X;
+const vec3& Vectors::UP = Vectors::UNIT_Y;
+const vec3& Vectors::FRONT = Vectors::UNIT_NEG_Z;
+
 //  Safe version of glm::mix; based on the code in Nick Bobick's article,
 //  http://www.gamasutra.com/features/19980703/quaternions_01.htm (via Clyde,
 //  https://github.com/threerings/clyde/blob/master/src/main/java/com/threerings/math/Quaternion.java)

libraries/shared/src/GLMHelpers.h

@@ -53,6 +53,28 @@ const glm::vec3 IDENTITY_FRONT = glm::vec3( 0.0f, 0.0f,-1.0f);
 
 glm::quat safeMix(const glm::quat& q1, const glm::quat& q2, float alpha);
 
+class Vectors {
+public:
+    static const vec3 UNIT_X;
+    static const vec3 UNIT_Y;
+    static const vec3 UNIT_Z;
+    static const vec3 UNIT_NEG_X;
+    static const vec3 UNIT_NEG_Y;
+    static const vec3 UNIT_NEG_Z;
+    static const vec3 UNIT_XY;
+    static const vec3 UNIT_XZ;
+    static const vec3 UNIT_YZ;
+    static const vec3 UNIT_ZX;
+    static const vec3 UNIT_XYZ;
+    static const vec3 MAX;
+    static const vec3 MIN;
+    static const vec3 ZERO;
+    static const vec3 ONE;
+    static const vec3& RIGHT;
+    static const vec3& UP;
+    static const vec3& FRONT;
+};
+
 // These pack/unpack functions are designed to start specific known types in as efficient a manner
 // as possible. Taking advantage of the known characteristics of the semantic types.
 

libraries/shared/src/LogHandler.cpp

@@ -38,14 +38,14 @@ LogHandler::LogHandler() :
 
 const char* stringForLogType(LogMsgType msgType) {
     switch (msgType) {
-        case QtDebugMsg:
+        case LogDebug:
             return "DEBUG";
-        case QtCriticalMsg:
+        case LogWarning:
-            return "CRITICAL";
-        case QtFatalMsg:
-            return "FATAL";
-        case QtWarningMsg:
             return "WARNING";
+        case LogCritical:
+            return "CRITICAL";
+        case LogFatal:
+            return "FATAL";
         case LogSuppressed:
             return "SUPPRESS";
         default:

libraries/shared/src/LogHandler.h

@@ -22,10 +22,10 @@
 const int VERBOSE_LOG_INTERVAL_SECONDS = 5;
 
 enum LogMsgType {
-    LogDebug,
+    LogDebug = QtDebugMsg,
-    LogWarning,
+    LogWarning = QtWarningMsg,
-    LogCritical,
+    LogCritical = QtCriticalMsg,
-    LogFatal,
+    LogFatal = QtFatalMsg,
     LogSuppressed
 };
 

libraries/shared/src/RingBufferHistory.h

@@ -174,7 +174,7 @@ public:
         }
 
         bool operator<=(const Iterator& rhs) {
-            return age() < rhs.age();
+            return age() <= rhs.age();
         }
 
         bool operator>=(const Iterator& rhs) {

libraries/shared/src/Transform.cpp

@@ -37,7 +37,7 @@ void Transform::evalRotationScale(Quat& rotation, Vec3& scale, const Mat3& rotat
             norm = (norm > n ? norm : n);
         }
         rotationMat = nextRotation;
-    } while (count < 100 && norm > ACCURACY_THREASHOLD);
+    } while (count++ < 100 && norm > ACCURACY_THREASHOLD);
 
 
     // extract scale of the matrix as the length of each axis

tools/scribe/src/TextTemplate.cpp

@@ -196,7 +196,7 @@ bool TextTemplate::grabUntilEndTag(std::istream* str, std::string& grabbed, Tag:
         preEnd = Tag::REM;
     }
 
-    while (!str->eof()) {
+    while (!str->eof() && !str->fail()) {
         // looking for the end of the tag means find the next preEnd
         std::string dataToken;
         getline((*str), dataToken, preEnd);
@@ -233,7 +233,7 @@ bool TextTemplate::stepForward(std::istream* str, std::string& grabbed, std::str
     if (grabUntilEndTag(str, tag, tagType)) {
         // skip trailing space and new lines only after Command or Remark tag block
         if ((tagType == Tag::COMMAND) || (tagType == Tag::REMARK)) {
-            while (!str->eof()) {
+            while (!str->eof() && !str->fail()) {
                 char c = str->peek();
                 if ((c == ' ') || (c == '\t') || (c == '\n')) {
                     str->get();