Valgrind и утечка памяти в CPP: «Условный переход или перемещение зависит от неинициализированных значений» - PullRequest
Новая
       126

Valgrind и утечка памяти в CPP: «Условный переход или перемещение зависит от неинициализированных значений»

1 голос
/ 21 июня 2020

Этот код компилируется и запускается, создавая ожидаемый результат, за исключением случаев, когда выполняется valgrind, тогда возникают эти утечки памяти. Следующий код запускается в Visual Studio без каких-либо предупреждений или ошибок.

Итак, мой вопрос: где происходит утечка памяти? Я относительно новичок в CPP и потратил на это несколько часов, поэтому эти ошибки застают меня врасплох.

Что-то я делаю не так с точки зрения последовательности? Я куда-то передаю неинициализированное значение? Запутался.

Мне сложно понять, где происходит потеря памяти. Вот файлы: 

/// Saiyan.cpp

#define _CRT_SECURE_NO_WARNINGS
#include <string.h>
#include <iostream>
#include "Saiyan.h"

using namespace std;

namespace sdds
{
    // CONSTRUCTORS:
    Saiyan::Saiyan()
    {
        // default state
        m_name = nullptr;   // Dynamic allocation:  set to nullptr!
        m_dob = 0;
        m_power = 0;
        m_super = false;
        m_level = 0;
    }
    
    Saiyan::Saiyan(const char* name, int dob, int power)
    {
        set(name, dob, power);
    }

    // MEMBER FUNCTIONS:
    void Saiyan::set(const char* name, int dob, int power, int level, bool super)
    {
        // Check if arguments are valid:
        if (name == nullptr || strlen(name) <= 0 || dob > 2020 || power <= 0)
        {
            *this = Saiyan();   // Calls constructor that creates default.
        }
        else
        {
            // Deallocate previosly allocated memory for m_name to avoid memory leak:
            if (m_name != nullptr && strlen(name) == 0)
            {
                delete[] m_name;
                m_name = nullptr;
            }
            // Assign validate values to current object:
            m_name = new char[strlen(name) + 1];
            strcpy(m_name, name);
            m_dob = dob;
            m_power = power;
            m_super = super;
            m_level = level;
        }
    }
    
    bool Saiyan::isValid() const
    {
        bool valid_state = m_name != nullptr && strlen(m_name) > 0 && m_dob < 2020 && m_power > 0;
        return valid_state;
    }

    void Saiyan::display() const
    {
        if (isValid())
        {
            cout << m_name << endl;
            
            cout.setf(ios::right);
            cout.width(10);
            cout << "DOB: " << m_dob << endl;
            cout.width(10);
            cout << "Power: " << m_power << endl;
            cout.width(10);
            if (m_super == true) {
                cout << "Super: " << "yes" << endl;
                cout.width(10);
                cout << "Level: " << m_level;
            }
            else
            {
                cout << "Super: " << "no";
            }
            cout.unsetf(ios::left);
        }
        else
        {
            cout << "Invalid Saiyan!";
        }
        cout << endl;
    }

    bool Saiyan::fight(Saiyan& other)
    {
        // Check both Saiyans for super level and power up accordingly:
        if (m_super == true)
        {
            m_power += int(m_power * (.1 * m_level));   // Cast an int to avoid possible memory loss.
        }
        if (other.m_super == true)
        {
            other.m_power += int(other.m_power * (.1 * other.m_level));
        }

        bool value = m_power > other.m_power;
        return value;
    }

    // DESTRUCTOR:
    Saiyan::~Saiyan()
    {
        if (m_name != nullptr)
        {
            delete[] m_name;    // Deallocate memory of member.
            m_name = nullptr;
        }
    }
}



// Saiyan.h

#pragma once
#ifndef SDDS_SAIYAN_H
#define SDDS_SAIYAN_H

namespace sdds
{
    class Saiyan
    {
        char* m_name;       // Dynamically allocated array of chars.
        int m_dob;          // Year the Saiyan was born.
        int m_power;        // Integer indicating the strength of the Saiyan (>= 0).
        bool m_super;       // indicates whether Saiyan can evolve
        int m_level;        // an integer indicating the level of a SS

        /*
        ***Valid Name*** : a dynamically allocated array of chars.
        ***Valid Year of Birth***: an integer within the interval[0, 2020].
        ***Valid Power***: an integer that is greater than 0.
        */

    public:
        Saiyan();
        Saiyan(const char* name, int dob, int power);  // Custom constructor
        void set(const char* name, int dob, int power, int level = 0, bool super = false);
        bool isValid() const;
        void display() const;
        bool fight(Saiyan& other);  // Fight and power up Saiyans.
        ~Saiyan();
    };
}

#endif



// main.cpp

#include <iostream>
#include "Saiyan.h"
#include "Saiyan.h"  // this is on purpose

using namespace std;
using namespace sdds;

void printHeader(const char* title)
{
    char oldFill = cout.fill('-');
    cout.width(40);
    cout << "" << endl;

    cout << "|> " << title << endl;

    cout.fill('-');
    cout.width(40);
    cout << "" << endl;
    cout.fill(oldFill);
}


int main()
{
    {
        printHeader("T1: Checking default constructor");

        Saiyan theSayan;
        theSayan.display();
        cout << endl;
    }

    {
        printHeader("T2: Checking custom constructor");

        Saiyan army[] = {
          Saiyan("Nappa", 2025, 1),
          Saiyan("Vegeta", 2018, -1),
          Saiyan("Goku", 1990, 200),
          Saiyan(nullptr, 2015, 1),
          Saiyan("", 2018, 5)
        };

        cout << "Only #2 should be valid:" << endl;
        for (int i = 0; i < 5; i++)
        {
            cout << "  Sayan #" << i << ": " << (army[i].isValid() ? "valid" : "invalid") << endl;
        }
        for (int i = 0; i < 5; i++)
        {
            army[i].display();
        }
        cout << endl;
    }

    // valid saiyans
    Saiyan s1("Goku", 1990, 2000);
    Saiyan s2;
    s2.set("Vegeta", 1989, 2200);

    {
        printHeader("T3: Checking the fight");
        s1.display();
        s2.display();

        cout << "S1 attacking S2, Battle " << (s1.fight(s2) ? "Won" : "Lost") << endl;
        cout << "S2 attacking S1, Battle " << (s2.fight(s1) ? "Won" : "Lost") << endl;
        cout << endl;
    }

    {
        printHeader("T4: Checking powerup");
        s1.set("Goku", 1990, 1900, 1, true);
        int round = 0;
        bool gokuWins = false;
        while (!gokuWins) // with every fight, the super saiyan should power up
        {
            cout << "Round #" << ++round << endl;
            gokuWins = s1.fight(s2);
            s1.display();
            s2.display();
        }

        cout << "Bonus round. Is s2 winning? " << (s2.fight(s1) ? "yes" : "no") << endl;
        s1.display();
        s2.display();
        cout << endl;
    }

    {
        printHeader("T5: Upgrading s2");
        s2.set("Vegeta", 1990, 2200, 3, true);

        cout << "Super Battle. Is s2 winning? " << (s2.fight(s1) ? "yes" : "no") << endl;
        s1.display();
        s2.display();
        cout << endl;
    }
 
    return 0;
}

Вот что в итоге заработало: 

/// Saiyan.cpp
#define _CRT_SECURE_NO_WARNINGS
#include <string.h>
#include <iostream>
#include "Saiyan.h"

using namespace std;

namespace sdds
{
    Saiyan::Saiyan()
    {
    }
    
    Saiyan::Saiyan(const char* name, int dob, int power)
    {
        set(name, dob, power);
    }

    void Saiyan::set(const char* name, int dob, int power, int level, bool super)
    {
        if (name != nullptr && name[0] != '\0')
        {
            if (m_name != nullptr)
            {
                delete[] m_name;
                m_name = nullptr;
            }
            m_name = new char[strlen(name) + 1];
            strcpy(m_name, name);
        }
        if (dob != 0 && dob < 2020)
        {
            m_dob = dob;
        }
        if (power > 0)
        {
            m_power = power;
        }
        if (level > 0)
        {
            m_level = level;
        }
        m_super = super;
    }
    
    bool Saiyan::isValid() const
    {
        bool valid_state = m_name != nullptr && m_dob != 0 && m_dob < 2020 && m_power > 0 && m_level >= 0;
        return valid_state;
    }

    void Saiyan::display() const
    {
        if (isValid())
        {
            cout << m_name << endl;
            
            cout.setf(ios::right);
            cout.width(10);
            cout << "DOB: " << m_dob << endl;
            cout.width(10);
            cout << "Power: " << m_power << endl;
            cout.width(10);
            if (m_super == true) {
                cout << "Super: " << "yes" << endl;
                cout.width(10);
                cout << "Level: " << m_level;
            }
            else
            {
                cout << "Super: " << "no";
            }
            cout.unsetf(ios::left);
        }
        else
        {
            cout << "Invalid Saiyan!";
        }
        cout << endl;
    }

    bool Saiyan::fight(Saiyan& other)
    {
        // Check both Saiyans for super level and power up accordingly:
        if (m_super == true)
        {
            m_power += int(m_power * (.1 * m_level));   // Cast an int to avoid possible memory loss.
        }
        if (other.m_super == true)
        {
            other.m_power += int(other.m_power * (.1 * other.m_level));
        }

        bool value = m_power > other.m_power;
        return value;
    }

    Saiyan::~Saiyan()
    {
        if (m_name != nullptr)
        {
            delete[] m_name;    // Deallocate memory of member.
            m_name = nullptr;
        }
    }
}



// Saiyan.h
#pragma once
#ifndef SDDS_SAIYAN_H
#define SDDS_SAIYAN_H

namespace sdds
{
    class Saiyan
    {
        char* m_name{};     // Dynamically allocated array of chars.
        int m_dob{};                // Year the Saiyan was born.
        int m_power{};          // Integer indicating the strength of the Saiyan (>= 0).
        bool m_super{};     // indicates whether Saiyan can evolve
        int m_level{};          // an integer indicating the level of a SS

        /*
        ***Valid Name*** : a dynamically allocated array of chars.
        ***Valid Year of Birth***: an integer within the interval[0, 2020].
        ***Valid Power***: an integer that is greater than 0.
        */

    public:
        Saiyan();
        Saiyan(const char* name, int dob, int power);  // Custom constructor
        void set(const char* name, int dob, int power, int level = 0, bool super = false);
        bool isValid() const;
        void display() const;
        bool fight(Saiyan& other);  // Fight and power up Saiyans.
        ~Saiyan();
    };
}

#endif

Ответы [ 2 ]

0 голосов
/ 26 июня 2020

Работает! 

    char* m_name = nullptr; // Dynamically allocated array of chars.
    int m_dob = 0;          // Year the Saiyan was born.
    int m_power = 0;        // Integer indicating the strength of the Saiyan (>= 0).
    bool m_super = false;   // indicates whether Saiyan can evolve
    int m_level = 0;        // an integer indicating the level of a SS
0 голосов
/ 21 июня 2020

В целом --- избегайте ручного управления памятью, почему бы просто не использовать std :: string?

Что касается проблем в коде.

Эта часть кода большая нет нет: 

        if (name == nullptr || strlen(name) <= 0 || dob > 2020 || power <= 0)
        {
            *this = Saiyan();   // Calls constructor that creates default.
        }

На самом деле вы здесь обходите деструктор, поэтому, если m_name было инициализировано, вы получите утечку памяти.

Другая проблема в этом конструкторе: 

    Saiyan::Saiyan(const char* name, int dob, int power)
    {
        set(name, dob, power);
    }

Вы не гарантирует, что ваш объект всегда будет в хорошем состоянии после вызова этого конструктора.

И последнее, но не менее важное, здесь: 

            if (m_name != nullptr && strlen(name) == 0)
            {
                delete[] m_name;
                m_name = nullptr;
            }

Вы освобождаете m_name только в том случае, если новое имя короткое, но вы должны освободить место независимо от длины нового имени, так как вы устанавливаете новое значение для m_name независимо от длины нового имени.

Также в C ++ 11 вы можете назначать значения по умолчанию для членов вне конструктора, и они будут использоваться в каждом конструкторе, в котором вы явно не устанавливаете другое значение: 

    class Saiyan
    {
        char* m_name = nullptr; // Dynamically allocated array of chars.
        int m_dob = 0;          // Year the Saiyan was born.
        int m_power = 0;        // Integer indicating the strength of the Saiyan (>= 0).
        bool m_super = false;   // indicates whether Saiyan can evolve
        int m_level = 0;        // an integer indicating the level of a SS

    public:
        Saiyan() {};
...
...