Program crashed while trying to delete[] a dynamically allocated array

Question

During one of my recent interviews, I was asked to implement and test a user defined version of memcpy() function in C++.

My code was something like this:

#include <iostream>

void my_memcpy(void* src, void* dest, size_t size)
{
    char* m_src = (char*)src;
    char* m_dest = (char*) dest;
    
    for (int i=0; i<size ; i++)
        *(m_dest+i) = *(m_src+i);
}

int main()
{
    char* source;
    char* destination;
    source = new char[20];
    destination = new char[20];
    
    source = "Hello";
    my_memcpy(source, destination,5);
    
    std::cout << destination << "\n";
    
    delete[] source;
    delete[] destination;

    return 0;
}

The program would run with a warning, but output correctly and then crash at the end. Output

When I commented out the delete[] source line, the program wasn't crashing anymore. I still don't understand why delete[] source would lead to a crash.

Kindly help me in explaining this or guide me to some reference which can clarify the underlying concept.

Does this answer your question? [Problems with char \* and delete](https://stackoverflow.com/questions/24245553/problems-with-char-and-delete) — Nico Schertler, Feb 05 '21 at 05:35
`source = "Hello";` shouldn't compile (from C++11 upwards, I think this part of the standard changed at one point?), because string literals are `char const[]`. — Lukas-T, Feb 05 '21 at 06:17
What did you learn when you ran your program using a memory checker (e.g. Valgrind)? — Toby Speight, Feb 05 '21 at 09:04

score 3 · Accepted Answer · edited Feb 05 '21 at 08:59

3

The program crashes because you aren't delete[]'ing the pointer you allocated with new[].

char* source;
source = new char[20];
source = "Hello";
delete[] source;

You first assign source to a dynamic array of 20 chars, but then you change source to point at a string literal. A string literal isn't allocated with new[], so delete[]'ing it crashes the program.

I suppose that when you wrote source = "Hello"; what you were trying to do was copy the characters of "Hello" into the location pointed at by source, but that's not what that line of code actually does. It copies the pointer, not what is being pointed at.

Here's one way of correcting the code:

strcpy(source, "Hello");

This would also work:

memcpy(source, "Hello", 5+1); // +1 for null terminator

Both of these functions copy the characters, not the pointer.

edited Feb 05 '21 at 08:59

Remy Lebeau

555,201
31
458
770

answered Feb 05 '21 at 06:15

john

85,011
4
57
81

3

This is a good explanation. But one should also take care of the fact that `"Hello"` is a 6 character literal because terminating `'\0'` is also a part of the string. Another caution need to be taken when working with strings. `source = new char[20];` only allocates memory which may already contain some data. Copying only 5 letters from `"Hello"` may leave the string unterminated and later reading it as a string may go beyond 20 chars allocated to it. – Ashutosh Raghuwanshi Feb 05 '21 at 07:26
Thanks @John for the explanation and Remy for the edit. I got my mistake now. – Kshitij_9192 Feb 06 '21 at 06:18

Program crashed while trying to delete[] a dynamically allocated array

1 Answers1