Why does the output fail to show the content of a variable after merely adding a cout line?

Question

#include <iostream>

using namespace std;

class Marks
{
public:
    char* name();
};

char* Marks::name()
{
    char temp[30];
    cout<<"Enter a name:"<<endl;
    cin.getline(temp,30);
    return temp;
}

int main ()
{
    char *name;
    Marks test1;
    name=test1.name();

    //cout<<"name:"; //uncomment this line to see the problem
    cout<<name<<endl;

    return 0;
}

Answer 1

You're returning a stack-based pointer (a pointer that resides in the stack of the called process) that will be released whenever the Marks::name() method ends.

This is basic C/C++ memory management question, so I encourage you to read some books on the topic.

There are several ways of doing this correctly. For example, reserving the memory for the string in the calling function and passing this pointer to the function:

In the main() method:

char name[30];
Marks test1;
test1.name(name);

(here name has the correct value) with the corresponding method:

char* Marks::name(char* temp)
{
    cout<<"Enter a name:"<<endl;
    cin.getline(temp,30);
    return temp;
}

But there are other ways of managing static and dynamic memory for strings.

Answer 2

The problem is because the value that name is pointing to has been destroyed. You are returning the address of a local variable from Marks::name(). Most likely a side affect of the first cout is causing the contents of name to be destroyed. You're probably just getting lucky when the first cout is commented out.

The correct way to do this is to allocate some memory, return that, and then destroy it when you're done:

char* Marks::name()
{
    char* temp = new char[30];
    cout<<"Enter a name:"<<endl;
    cin.getline(temp,30);
    return temp;
}

int main ()
{
    char *name;
    Marks test1;
    name=test1.name();

    cout<<"name:";
    cout<<name<<endl;

    delete[] name;

    return 0;
}

Don't forget to use delete[], rather than just delete, since otherwise only the first character would be deallocated.

Answer 3

Well, you have a pretty nasty bug in Marks::name() where you return a pointer to a temporary variable. temp will be destroyed at the exit of the function as it's going out of scope, so you are returning a pointer to a variable that doesn't exist anymore.

Answer 4

You are returning address of a local variable:

char temp[30];
// ...
return temp;

This is a strict no-no in C or C++. The moment you come out of Marks::name() your temp variable goes BOOM! You can no longer access it -- doing that invokes Undefined Behavior. Try this,

#include <string> // somewhere at the top
// ...
std::string Marks::name()
{
   std::string line;
   cout<<"Enter a name:"<<endl;
   std::getline(cout, line);
   return line;
}

Answer 5

All the answers so far are correct, but some of the ways of implementing this could have been better. I know its a very simple test application, but consider the following:

char* Marks::name()
{
    char* temp = new char[30];
    cout<<"Enter a name:"<<endl;
    cin.getline(temp,30);
    return temp;
}

int main ()
{
    char *name;
    Marks test1;
    name=test1.name();

    cout<<"name:";
    cout<<name<<endl;

    delete[] name;

    return 0;
}

Here, it is the function name() which is allocating the memory, but it is not deleting it. This is an inconsistancy which can lead to problems, especially in a more complicated senario. It might be better to do this (assuming you know ahead of time how much space you are going to give people to write in...)

//this time, pass in some ALREADY allocated memory, and a length count.
void Marks::name(char *buf, int len)
{
    cout<<"Enter a name (max " << len <<" letters):"<<endl;
    cin.getline(buf,len); //read into the memory provided
}

int main ()
{
    //preallocate your memory so that the same code that allocates...
    char *name = new char[30];
    Marks test1;
    test1.name(name, 30);

    cout<<"name:";
    cout<<name<<endl;

    //... also deallocates it. This is perhaps cleaner in terms of responcibility.
    delete[] name;

    return 0;
}

In reality there is no right or wrong way to do this kind of thing. Sometimes a function has to allocate memory to use and return this for other applications to use, this is just an example where you should think about who is newing and try to make it consistant when coming to delete.

Answer 6

Marks::name() returns a pointer to a local variable. That memory will be freed when the function exits.

The reason you still get the right output as long as you don't call any functions in between your calls to Marks::name and cout::operator << is that no other allocation claims that memory area.

You could use std::string instead of a char* if you intend to return a non-static string by value.

Alternatively, you could pass the char buffer to the name function as a char* and also pass the buffer's size and have Marks::name fill that buffer (e.g. using strncpy or strncpy_s)

void Marks::name( char* buffer, int length )
{
  cout<<"Enter a name:"<<endl;
  cin.getline(buffer,length);
}

void main()
{
  char buffer[30];
  Marks::name( buffer, 30 );
}