0

I helped my friend with debugging a code, problematic part was something like that:

class MyClass {
    char * text;
public:
    MyClass(const char * c) {
        if (c != nullptr) {
            text = new char[strlen(c)];
            strcpy(text, c);
        }
        else
            text = nullptr;
    }
    ~MyClass() {
        delete[] text;
    }
};


int main() {
    MyClass foo("bar");
    return 0;
}

Of course problem is with strlen(c), should be strlen(c) + 1. Anyway, what surprised me, why did that result in heap corruption error in the moment of calling delete[] in destructor? What caused it?

This error is thrown by debugger, my question is: Why did this error popped up at the moment of freeing memory, not earlier? It would be much easier to find any bugs in code this way.

@edit old c = nullptr -> text = nullptr There was this bug I wrote accidentally, (sorry, I didn't notice, now it is what I meant). The thing is, it was forbidden to use strings in this task, so it had to be done C-way. Sorry for so many edits. I really have to learn how to ask precise questions.

Kaznov
  • 1,035
  • 10
  • 17
  • 11
    Most likely you violated the [rule of 3](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – NathanOliver Mar 23 '17 at 19:26
  • 1
    @someprogrammerdude "*Of course problem is with strlen(c), should be strlen(c) + 1.*". – Hatted Rooster Mar 23 '17 at 19:28
  • 5
    `c = nullptr` you probably mean `text = nullptr` there. .. – Daniel Jour Mar 23 '17 at 19:28
  • 2
    Your example is just a class definition; there are no `MyClass` objects created, so the destructor is not called. Post an MCVE please. – Christian Hackl Mar 23 '17 at 19:28
  • 5
    When you go out of bounds with the `strcpy` call you have *undefined behavior*. You can't really say what that will lead to, but most likely you overwrite some data used by the memory allocator leading to problems when you `delete[]`. – Some programmer dude Mar 23 '17 at 19:29
  • 1
    The `delete` in question is the cause of the message, not of the corruption. The memory manager checks before releasing memory that things are OK. When something isn't, it was usually caused a long time ago in a different part of the code. – molbdnilo Mar 23 '17 at 19:43
  • 2
    Let's see... 1) using `char*` instead of `std::string` 2) not allocating enough memory for the null terminator 3) not handling the rule of three 4) assigning the constructor argument to null instead of the member variable. Did I miss any? – Fred Larson Mar 23 '17 at 19:43
  • 1
    My advice: Understand and then fix the bug @NathanOliver mentioned. – drescherjm Mar 23 '17 at 19:51
  • It is just a part of code. I didn't see need to paste also copying operator, as in this case it doesn't matter, and makes code unnecessarily long. – Kaznov Mar 23 '17 at 19:59
  • 1
    ***Why did this error popped up at the moment of freeing memory, not earlier?*** From experience heap corruption is not necessarily detected in Visual Studio when the corruption occurs. You can't rely on detection happening just after the corruption. Although in this case the corruption was detected at the next possible time to detect the corruption. Visual Studio only checks when you allocate or free memory. – drescherjm Mar 23 '17 at 20:18
  • @drescherjm Thanks, this was exactly the answer I was looking for from the beginning! – Kaznov Mar 23 '17 at 20:51

2 Answers2

3

The heap corruption was caused by the call to strcpy, which wrote past the end of the allocated memory block. It was detected when the code called delete[].

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
1

Why did this error popped up at the moment of freeing memory, not earlier?

From experience heap corruption is not necessarily detected in Visual Studio when the corruption occurs. You can't rely on detection happening just after the corruption. Although in this case the corruption was detected at the next possible time to detect the corruption. Visual Studio only checks when you allocate or free memory.

drescherjm
  • 10,365
  • 5
  • 44
  • 64