Need help finding a memory leak

Question

I am doing an assignment for school and I have provided the code below and I keep getting a memory leak but I don't understand why its happening. The program used to find the memory leak is valgrind. Note: I cant change the main file it was provided by the school and used to mark us.

Saiyan.cpp

#include <iomanip>
#include <cstring>
#include "Saiyan.h"

using namespace std;

namespace sdds
{
    Saiyan::Saiyan()
    {
        m_name = nullptr;
        m_dob = 0;
        m_power = -1;
        m_level = -1;
        m_super = false;
    }
    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 || strlen(name) == 0 || dob > 2020 || power <= 0 || level < 0)
        {
            
            this->m_name = nullptr;
            this->m_dob = 0;
            this->m_power = -1;
            this->m_level = -1;
            this->m_super = false;
        }
        else
        {
            int len = strlen(name);
            m_name = new char[len + 1];
            strcpy(this->m_name, name);
            this->m_name[len] = '\0';
            this->m_dob = dob;
            this->m_power = power;
            this->m_super = super;
            this->m_level = level;
        }
    }
    bool Saiyan::isValid() const
    {
        return this->m_name != nullptr;
    }
    void Saiyan::display() const
    {
        
        if (isValid())
        {
            cout << this->m_name << endl;
            cout.width(10);
            cout << "DOB: " << m_dob << endl;
            cout.width(10);
            cout << "Power: " << m_power << endl;
            cout.width(10);
            cout << "Super: ";
            if (m_super)
            {
                cout << "yes" << endl;
                cout.width(10);
                cout << "Level: " << m_level << endl;
            }
            else
            {
                cout << "no" << endl;
            }
        }
        else
        {
            cout << "Invalid Saiyan!" << endl;
        }
    }
    bool Saiyan::fight(Saiyan& other)
    {
        bool cond = false;
        if (isValid() && other.isValid())
        {
            if (this->m_super)
            {
                this->m_power += (int)((10/100) *this->m_level* this->m_power);
            }
            if (other.m_super)
            {
                other.m_power += (int)((10/100) * other.m_level * other.m_power);
            }

            if (m_power > other.m_power)
            {
                cond = true;
            }
        }
        return cond;
    }
    Saiyan::~Saiyan()
    {
        delete [] this->m_name;
    }
}

Saiyan.h

#ifndef SDDS_SAIYAN_H
#define SDDS_SAIYAN_H


namespace sdds
{
    class Saiyan
    {
        char* m_name;
        int m_dob;
        int m_power;
        int m_level;
        bool m_super;
    public:
        Saiyan();
        ~Saiyan();
        Saiyan(const char* name, int dob, int power);
        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);

    };
}

#endif // !SDDS_SAIYAN_H

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;
}

The memory leak

==211503==
==211503== HEAP SUMMARY:
==211503==     in use at exit: 12 bytes in 2 blocks
==211503==   total heap usage: 5 allocs, 3 frees, 29 bytes allocated
==211503==
==211503== 5 bytes in 1 blocks are definitely lost in loss record 1 of 2
==211503==    at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==211503==    by 0x400BC1: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:38)
==211503==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==211503==    by 0x401241: main (main_prof.cpp:66)
==211503==
==211503== 7 bytes in 1 blocks are definitely lost in loss record 2 of 2
==211503==    at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==211503==    by 0x400BC1: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:38)
==211503==    by 0x401274: main (main_prof.cpp:68)
==211503==
==211503== LEAK SUMMARY:
==211503==    definitely lost: 12 bytes in 2 blocks
==211503==    indirectly lost: 0 bytes in 0 blocks
==211503==      possibly lost: 0 bytes in 0 blocks
==211503==    still reachable: 0 bytes in 0 blocks
==211503==         suppressed: 0 bytes in 0 blocks

Edit: After doing what was suggested I add deallocated the memory but these errors came up.

New Errors:

==218753== Conditional jump or move depends on uninitialised value(s)
==218753==    at 0x400B77: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:29)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x4010DC: main (main_prof.cpp:51)
==218753==
==218753== Conditional jump or move depends on uninitialised value(s)
==218753==    at 0x400B77: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:29)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x4010FC: main (main_prof.cpp:51)
==218753==
==218753== Conditional jump or move depends on uninitialised value(s)
==218753==    at 0x400BDA: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:41)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x40111B: main (main_prof.cpp:51)
==218753==
==218753== Conditional jump or move depends on uninitialised value(s)
==218753==    at 0x400B77: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:29)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x40113A: main (main_prof.cpp:51)
==218753==
==218753== Conditional jump or move depends on uninitialised value(s)
==218753==    at 0x400B77: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:29)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x401159: main (main_prof.cpp:51)

==218753== Conditional jump or move depends on uninitialised value(s)
==218753==    at 0x400BDA: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:41)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x401285: main (main_prof.cpp:66)
==218753==
==218753== Conditional jump or move depends on uninitialised value(s)
==218753==    at 0x4C2BB42: operator delete[](void*) (vg_replace_malloc.c:651)
==218753==    by 0x400BEA: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:41)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x401285: main (main_prof.cpp:66)
==218753==
==218753== Invalid free() / delete / delete[] / realloc()
==218753==    at 0x4C2BB8F: operator delete[](void*) (vg_replace_malloc.c:651)
==218753==    by 0x400BEA: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:41)
==218753==    by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20)
==218753==    by 0x401285: main (main_prof.cpp:66)
==218753==  Address 0x1ffefffec0 is on thread 1's stack
==218753==  in frame #3, created by main (main_prof.cpp:33)

program edit:

 void Saiyan::set(const char* name, int dob, int power,int level, bool super)
        {

                if (!name || strlen(name) == 0 || dob > 2020 || power <= 0 || level < 0)
                {
                        delete [] this->m_name;
                        this->m_name = nullptr;
                        this->m_dob = 0;
                        this->m_power = -1;
                        this->m_level = -1;
                        this->m_super = false;
                }
                else
                {


                        int len = strlen(name);
                        delete [] this->m_name;
                        m_name = new char[len + 1];
                        strcpy(this->m_name, name);
                        this->m_name[len] = '\0';
                        this->m_dob = dob;
                        this->m_power = power;
                        this->m_super = super;
                        this->m_level = level;
                }
        }

Save yourself a ton of effort, and ditch the whole `char[]` and replace it with `std::string`. Your program will be simpler to write, and you won't have issues like this. — Tas, Jun 22 '20 at 05:03
I would love to but I cant, my teacher wants me to use char. — Muhammad Faiz, Jun 22 '20 at 05:05

score 0 · Accepted Answer · answered Jun 22 '20 at 05:26

0

you need to delete the m_name in set function before reusing it:

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

answered Jun 22 '20 at 05:26

yaodav

1,126
12
34

hi, so I did what you said but I started getting more errors; ==218753== Conditional jump or move depends on uninitialised value(s) ==218753== at 0x400BDA: sdds::Saiyan::set(char const*, int, int, int, bool) (Saiyan.cpp:41) ==218753== by 0x400B23: sdds::Saiyan::Saiyan(char const*, int, int) (Saiyan.cpp:20) ==218753== by 0x401285: main (main_prof.cpp:66) ==218753== – Muhammad Faiz Jun 22 '20 at 05:31
All the memory was released tho btw. its just that these new errors came up and im using valgrind. – Muhammad Faiz Jun 22 '20 at 05:34
also if the answer solved your problem accept it so other people know that it is a good answer – yaodav Jun 22 '20 at 05:39
read [this](https://stackoverflow.com/questions/2612447/pinpointing-conditional-jump-or-move-depends-on-uninitialized-values-valgrin/2612524) it will help you soleve the new errors – yaodav Jun 22 '20 at 05:41

Need help finding a memory leak

1 Answers1