-2

What is the way arround HEAP CORRUPTION DETECTED in my code? I am trying to write a basic program using c++ object oriented programming. My code looks like this:

#define _CRT_SECURE_NO_WARNINGS
#include <iostream>

using namespace std;

class Plant
{
public:
    Plant(void);                   // Constructor for the Plant class
    Plant(const char*);            // Overloaded constructor for the Plant class
    Plant(Plant&);                 // Copy constructor for the Plant class
    ~Plant(void)                   // Destructor for the Plant class
    {
        delete kingdom;
    }
    virtual void print(void);      // Virtual function to print information about the Plant
    void link(Plant*);             // Function to link a Plant object to the current object
    void printList(Plant*);        // Function to print a list of Plant objects
protected:
    char* kingdom;                 // Pointer to a character array representing the plant kingdom
    Plant* next;                   // Pointer to the next Plant object in the list
};

class Flower : public Plant       // Derived class Flower inheriting from the Plant class
{
public:
    Flower(const char* r = " division - Angiosperms");  // Constructor for the Flower class
    ~Flower(void)                  // Destructor for the Flower class
    {
        delete division;
    }
    virtual void print(void);      // Virtual function to print information about the Flower
private:
    char* division;                // Pointer to a character array representing the flower division
};

class ClassFlower : public Flower  // Derived class ClassFlower inheriting from the Flower class
{
public:
    ClassFlower(const char* v = "Monocotyledons");  // Constructor for the ClassFlower class
    ~ClassFlower(void)             // Destructor for the ClassFlower class
    {
        delete classFlower;
    }
    virtual void print(void);      // Virtual function to print information about the ClassFlower
private:
    char* classFlower;             // Pointer to a character array representing the flower class
};

// Definitions of class Plant member functions

Plant::Plant(void)
{
    next = 0;
    kingdom = new char[30];
    strcpy(kingdom, " the most diverse kingdom of species");
}

Plant::Plant(const char* newKingdom)
{
    next = 0;
    kingdom = new char[strlen(newKingdom) + 1];
    strcpy(kingdom, newKingdom);
}

Plant::Plant(Plant& newPlant)
{
    next = 0;
    kingdom = new char[strlen(newPlant.kingdom) + 1];
    strcpy(kingdom, newPlant.kingdom);
}

void Plant::print(void)
{
    cout << "The plant kingdom is " << kingdom << "\n";
}

void Plant::link(Plant* pt)
{
    pt->next = next;
    next = pt;
}

void Plant::printList(Plant* first)
{
    for (Plant* np = first; np; np = np->next)
        np->print();
}

// Definitions of class Flower member functions

Flower::Flower(const char* r)
{
    division = new char[strlen(r) + 1];
    strcpy(division, r);
}

void Flower::print(void)
{
    cout << "Roses are from" << division << "\n";
}

// Definitions of class ClassFlower member functions

ClassFlower::ClassFlower(const char* r)
{
    classFlower = new char[strlen(r) + 1];
    strcpy(classFlower, r);
}

void ClassFlower::print(void)
{
    Flower::print();
    cout << "from class " << classFlower << "\n";
    void getch();
}

// main function

void main(void)
{
    Plant* p;
    Plant* p1;
    Plant* a = new Plant;         // Create a Plant object using the default constructor
    Plant* u = new Flower;        // Create a Flower object using the default constructor
    Plant* m = new ClassFlower;   // Create a ClassFlower object using the default constructor

    p = a;
    Plant* list = p;
    p1 = u;
    p->link(p1);
    p = p1;
    p1 = m;
    p->link(p1);

    list->printList(list);        // Print the list of Plant objects
    delete a;                     // Free the memory for the Plant object
    //delete u;                     // Free the memory for the Flower object
    //delete m;                     // Free the memory for the ClassFlower object
    delete p1;
}

I get the error HEAP CORRUPTION DETECTED: after Normal block (#159) at 0x000001F2C954FC30. It is because I am not sure which object should be deleted at the end of main. I would appriciate any help and guidance since I am a novice at programming.

I tried deleting only a variable, and then all of the pointer variables but the error remains.

mch
  • 9,424
  • 2
  • 28
  • 42
  • 5
    Stop using `using namespace std;` Use a debugger, don't use new/delete (but [std::make_unique](https://en.cppreference.com/w/cpp/memory/unique_ptr/make_unique)). For strings don't use `char*` but [std::string](https://en.cppreference.com/w/cpp/string/basic_string)... (avoid strcpy, memcpy) all in all I would say learn a bit more C++ first (you still use a lot of "C" style coding, so either you are used to "C" or are learning C++ from an outdated source) – Pepijn Kramer May 30 '23 at 10:52
  • 1
    Make `~Plant` destructor virtual. At least gcc12 warns about it if you add proper flags - `-pedantic -Wall -Wextra`. – Quimby May 30 '23 at 10:53
  • 2
    Learn C++ from : [a recent C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) or from [learncpp.com](https://www.learncpp.com/) that site is decent and pretty up to date. Then use [cppreference](https://en.cppreference.com/w/) for reference material (and examples). When you have done all that keep using [C++ core guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) to keep up to date with the latest best practices since C++ keeps evolving. – Pepijn Kramer May 30 '23 at 10:53
  • `new T[n]` should be paired with `delete[]`, not `delete` as you're using. – interjay May 30 '23 at 10:56
  • 1
    Use warnings as errors and compiler immediately finds the problem: https://godbolt.org/z/8hKeY4roM `forming offset [30, 36] is out of the bounds [0, 30]` – Marek R May 30 '23 at 11:05
  • I don't understand the `next` pointer in `Plant`. `Plant` should be a plant and nothing more. Your code should be singular in purpose (That's 'S' from the SOLID principles, and should have been a programming 101 lesson). You can have a plant, and you can have a list. What you should **not** have is a plant *that is also* a list. If you need a list of polymorphic plants, use `std::list>`. I also agree with the other comments recommending C++. This code is minimal C++, and a lot of C. – sweenish May 30 '23 at 12:59
  • Use of proper C++ containers would completely remove all of your manual memory management in this case. – sweenish May 30 '23 at 13:07
  • And I didn't catch this in the "C" of other things, but the code doesn't even compile for me becuase your `main()` doesn't return an `int`. – sweenish May 30 '23 at 13:09
  • And here's my error text: `a.out(8514,0x7ff848239340) malloc: Incorrect checksum for freed object 0x7fb822f05bc0: probably modified after being freed.` – sweenish May 30 '23 at 13:11
  • @Krasimir The answer you've gotten seems to answer your question (and a lot more) well. If you don't find the answer acceptable in its current form, just ask for clarifications in the comment section under the answer. – Ted Lyngmo Jun 22 '23 at 16:20

1 Answers1

2

I'm going to put this together from my comments. But the gist is that C++ is not C, and you're doing many things (incorrectly) in a C fashion, but the C++ way completely removes the errors for you.

The issue is likely in your linked list code, but I'm not going to bother debugging it, as you should not have integrated it into your Plant class. Your code should have a single purpose. If you need a Plant class, only write Plant code for it. If you need a linked list, use std::list from <list>. Avoid C-strings and all the headache that comes from managing them manually and use std::string from <string>. This code does deal with heap memory, but you can avoid new and delete and use std::unique_ptr from <memory>.

There are other issues, like your base class destructor not being virtual. And minor nits like leaving () empty instead of writing void. That's a C-ism, and unnecessary in C++.

using namespace std; isn't saving that much time or typing, so start learning to stop using it. It will only cause headaches as the code you write gets bigger and more complex.

Other nits are less important, but contribute to better styled code. For example, protected member data is a bad idea. Issues with that data could come from any derived class, or the base. The data should be private, and getters and setters should be protected. Issues with the data lie in a single class.

And with constructors, make use of the initialization section.

Here is an example of your code with more C++ applied to it. It still uses heap memory, but has zero news and deletes because the Standard Library constructs employed do this for you.

#include <iostream>
#include <list>
#include <memory>
#include <string>

class Plant {
 public:
  Plant();
  Plant(std::string const &);
  virtual ~Plant() = default;
  virtual void print() const;

 protected:
  std::string get_kingdom() const;

 private:
  std::string kingdom;
};

class Flower : public Plant {
 public:
  Flower(std::string const &r = " division - Angiosperms");
  virtual void print() const override;

 private:
  std::string division;
};

class ClassFlower : public Flower {
 public:
  ClassFlower(std::string const &v = "Monocotyledons");

  void print() const override;

 private:
  std::string classFlower;
};

// Definitions of class Plant member functions
Plant::Plant() : kingdom(" the most diverse kingdom of species") {}

Plant::Plant(std::string const &newKingdom) : kingdom(newKingdom) {}

void Plant::print() const {
  std::cout << "The plant kingdom is" << kingdom << "\n";
}

std::string Plant::get_kingdom() const { return kingdom; }

// Definitions of class Flower member functions
Flower::Flower(std::string const &r) : division(r) {}

void Flower::print() const {
  std::cout << "Roses are from" << division << "\n";
}

// Definitions of class ClassFlower member functions
ClassFlower::ClassFlower(std::string const &r) : classFlower(r) {}

void ClassFlower::print() const {
  Flower::print();
  std::cout << "from class " << classFlower << "\n";
}

// main function

int main() {
  std::list<std::unique_ptr<Plant>> plantList;

  plantList.emplace_back(std::make_unique<Plant>());
  plantList.emplace_back(std::make_unique<Flower>());
  plantList.emplace_back(std::make_unique<ClassFlower>());

  for (auto const &plant : plantList) {
    plant->print();
    std::cout << '\n';
  }
}
sweenish
  • 4,793
  • 3
  • 12
  • 23