What is wrong with this code?

Question

int main()
{
    char *name = new char[7];
    name = "Dolphin";
    cout << "Your name is : " << name <<endl;
    delete [] name;
}

Why doesn't the VC++ compiler complain?

Answer 1

C-style strings have to be null terminated ("Dolphin\0"). The compiler will do this for you, but you didn't give it enough space. Change it to :

char *name = new char[8];

Answer 2

You've got two questions here:

First, What's wrong with code? Well ...

When you assign "Dolphin" to name you are not copying into the allocated array you are adjusting the pointer to point to a string literal. Later you try to delete what the pointer points to. I'd expect this to crash horribly in some environments.

If you really want a copy of the "Dolphin" characters, have a look at strncpy(), but as already been observed, you need a space for the null too.

Second, why that particular compiler doesn't warn you that the assignment is potentially: that's a bit harder. [It's been observed that other compilers will give a warning.] The question is whether this compiler treats a string literal as a "Pointer to const char" or a "Pointer to char".

If it was the former case then I'd expect an error. Until about 2004 C++ was consistent with C in treating literals as pointer to char, and hence the assignment would be permitted. So I guess the question for you is to determine what version of the specs you are working against, and that might depend upon the version of VC++ you are using and also any compiler options you have chosen.

A MSDN C++ reference indicates that VC++ treats string literals as non const. I'll leave it to VC++ gurus to comment further.

Answer 3

1. You need to allocate 8 characters
2. You can't assign a string literal like that. Either use const char *name = "Dolphin" (no need to assign!) and DO NOT DELETE or use strcpy to copy the text into the newly allocated space - which you later delete.

You cannot delete a string literal ("Dolphin") which resides in read only memory. Also on that note - when you write name = "Dolphin" you reassign the pointer which means that the memory initially allocated is lost.

Answer 4

This line is bad:

name = "Dolphin";

That is not copying a string but reassigning the pointer.

So this causes two problems.

1. You are freeing non allocated memory here delete [] name; because you are trying to free the pointer to the literal string "Dolphin"
2. The memory allocated in char *name = new char[7]; is leaked.

Since this is C++ you should just use std::string. No need to manually allocate or deallocate and then you get value semantics:

int main()
{
    std::string name;
    name = "Dolphin";
    cout << "Your name is : " << name <<endl;
}

Finally, if you are trying to understand raw pointers better, here is a version of your code that will work correctly. The important things to note is that the code allocates the length+1 (the extra +1 is for the terminating NUL byte) and uses strcpy to copy the data into the newly allocated memory:

char *strdup_via_new(const char *str)
{
    char *tmp = new char[strlen(str) + 1];
    strcpy(tmp, str);
    return tmp;
}

int main()
{
    char *name = strdup_via_new("Dolphin");
    cout << "Your name is : " << name <<endl;
    delete [] name;
}

Answer 5

The problem is that you're trying to delete a string constant, "Dolphin".

Answer 6

Is the assignment specifically to use new and delete to prove you know how? In that case you need to save room for the null terminator and use strcpy. If not, perhaps you're supposed to be demonstrating initializing?

Answer 7

1. You have not included iostream, and stated that you're using namespace std; while you are using cout and cin, defined therein,
2. You have a memory leak in line 4. name points to some memory, if you assign a string literal, you are actually assigning a pointer to it, thus losing the pointer to the 7 bytes allocated earlier,
3. In the 6th line you are trying to free a block of memory, which presumably is in read-only memory, as it was prepared by the compiler from a string literal. You are not allowed to free such memory by the standard.

Two correct ways of doing this would be:

#include <iostream>
#include <cstring>
using namespace std;
int main()
{
    char *name = new char[8];
    strcpy(name, "Dolphin");
    cout << "Your name is : " << name <<endl;
    delete [] name;
    return 0;
}

Here, remember that to store "Dolphin" you need 8 bytes, 1 goes for the NUL character. The second version (more C++):

#include <iostream>
#include <string>
using namespace std;
int main()
{
    string name = "Dolphin";
    cout << "Your name is : " << name <<endl;
    return 0;
}

And this one doesn't need any deleting.

The reason the compiler doesn't complain is simply that the compiler does not check for memory leaks for you. There are other programs to do that, valgrind for example. The compiler won't complain about deleting the string literal's memory either, because it doesn't analyse what you have done to the pointer since you assigned the address of a string literal to it, so it can't know it hasn't changed since then.

Answer 8

const char* name;

is sufficient. When you assign "Dolphin" to name, the value of the pointer changes. Finally, the delete[] name is wrong. It tries to delete the constant char* "Dolphin" which is not allocated on the heap.

So, the entire code could read

int main()
{
    const char *name;
    name = "Dolphin";
    cout << "Your name is : " << name <<endl;
}

The compiler doesn't complain since your code didn't violate C++'s grammar

Answer 9

This problem with this code is starting the line:

name = "Dolphin";

which is fine, since it assigns the variable name to point to string constant "Dolphin", but the line:

delete [] name;

tries to delete the string constant, which is undefined.

You should use something like std::string to handle your strings.

VC++ doesn't complain because it is C++, the language expects that you know what you are doing.

"C makes it easy to shoot yourself in the foot. C++ makes it harder, but when you do, it blows away your whole leg." - Bjarne Stroustrup

Answer 10

may be because VC++ thinks that , that's what you want to do in program. there is hardly any syntax error. so its ok. may linker call for memory leak errors ?

do compiler check for logic too these days ?

Answer 11

This source code exemplifies the difference between an ill-formed program and a program with logic errors (errors that cause a program to operate incorrectly). C++ compilers are only required to "complain" about an ill-formed program, which this source code is not.

The logic errors in this program include:

1. A memory leak caused by the line name = "Dolphin"
2. Attempting to call delete[] on a pointer that was not returned by new[]

Also, though you didn't copy "Dolphin" into the 7-char array, it appears that you intended to do this. You need to be careful that the destination allocation has space for the eight characters that make up the string "Dolphin" ('D', 'o', 'l', 'p', 'h', 'i', 'n', and '\0').

Answer 12

Whats wrong with the code? A. It has a memory leak. B. It does an unnecessary new

char *name = new char[7];

Is not needed since you are eventually going to assign name to a statically allocated string

name = "Dolphin";

C. Even if it actually did what it was intended to do.. it should have been char[8] and not char[7]. This is inviting seg faults

Why doesn't VC++ compiler complain? Compilers aren't really intended to complain about memory leaks and logical errors.

HTH