CODEWITHSUNDEEP
c++
Mark
Mark

Reputation: 2195

How to have a char pointer as an out parameter for C++ function

I'm a newbie to C++. I'm trying to have a char pointer as an out parameter for a function. But the changes made in the function are not reflected in the main function. What am I doing wrong?

void SetName( char *pszStr )
{
    char* pTemp = new char[10];
    strcpy(pTemp,"Mark");
    pszStr = pTemp;
}

int _tmain(int argc, _TCHAR* argv[])
{
    char* pszName = NULL;
    SetName( pszName );
    cout<<"Name - "<<*pszName<<endl;
    delete pszName;
    return 0;
}

Upvotes: 13

Views: 31069

Answers (4)

GManNickG
GManNickG

Reputation: 503973

Your pointer is being copied onto the stack, and you're assigning the stack pointer. You need to pass a pointer-to-pointer if you want to change the pointer:

void SetName( char **pszStr )
{
    char* pTemp = new char[10];
    strcpy(pTemp,"Mark");
    *pszStr = pTemp; // assign the address of the pointer to this char pointer
}

int _tmain(int argc, _TCHAR* argv[])
{
    char* pszName = NULL;
    SetName( &pszName ); // pass the address of this pointer so it can change
    cout<<"Name - "<<*pszName<<endl;
    delete pszName;
    return 0;
}

That will solve your problem.

However, there are other problems here. Firstly, you are dereferencing your pointer before you print. This is incorrect, your pointer is a pointer to an array of characters, so you want to print out the entire array:

cout<<"Name - "<<pszName<<endl;

What you have now will just print the first character. Also, you need to use delete [] to delete an array:

delete [] pszName;

Bigger problems, though, are in your design.

That code is C, not C++, and even then it's not standard. Firstly, the function you're looking for is main:

int main( int argc, char * argv[] )

Secondly, you should use references instead of pointers:

void SetName(char *& pszStr )
{
    char* pTemp = new char[10];
    strcpy(pTemp,"Mark");
    pszStr = pTemp; // this works because pxzStr *is* the pointer in main
}

int main( int argc, char * argv[] )
{
    char* pszName = NULL;
    SetName( pszName ); // pass the pointer into the function, using a reference
    cout<<"Name - "<<pszName<<endl;
    delete pszName;
    return 0;
}

Aside from that, it's usually better to just return things if you can:

char *SetName(void)
{
    char* pTemp = new char[10];
    strcpy(pTemp,"Mark");
    return pTemp;
}

int main( int argc, char * argv[] )
{
    char* pszName = NULL;
    pszName = SetName(); // assign the pointer
    cout<<"Name - "<<pszName<<endl;
    delete pszName;
    return 0;
}

There is something that makes this all better. C++ has a string class:

std::string SetName(void)
{
    return "Mark";
}

int main( int argc, char * argv[] )
{
    std::string name;

    name = SetName(); // assign the pointer

    cout<<"Name - "<< name<<endl;

    // no need to manually delete
    return 0;
}

If course this can all be simplified, if you want:

#include <iostream>
#include <string>

std::string get_name(void)
{
    return "Mark";
}

int main(void)
{
    std::cout << "Name - " << get_name() << std::endl;        
}

You should work on your formatting to make things more readable. Spaces inbetween your operators helps:

cout<<"Name - "<<pszName<<endl;

cout << "Name - " << pszName << endl;

Just like spaces in between English words helps, sodoesspacesbetweenyouroperators. :)

Upvotes: 56

Kei
Kei

Reputation: 1616

You can also use a reference to a pointer int this case. Also, you may want to be aware of 2 other bugs which are in the original code (see my comments in the code snippet).

 void SetName( char *& pszStr )
 {
     char* pTemp = new char[10];
     strcpy(pTemp,"Mark");
     pszStr = pTemp;
 }

 int _tmain(int argc, _TCHAR* argv[])
 {
     char* pszName = NULL;
     SetName(pszName);

     // Don't need '*' in front of pszName.
     cout<< "Name - " << pszName << endl;

     // Needs '[]' to delete an array.
     delete[] pszName;
     return 0;
 }

Upvotes: 8

Brian R. Bondy
Brian R. Bondy

Reputation: 347296

Since you tagged as C++, why not pass in a std::string reference and fill it?

void GetName(std::string &strName)
{
    strName = "Mark";
}

Or simply return an std::string:

std::string GetName2()
{
  return "Mark";
}

And call it like so

std::string strName, strName2;
GetName(strName);
strName2 = GetName2();
assert(strName == "Mark");
assert(strName2 == "Mark");
//strName.c_str() returns the const char * pointer.

Then you don't have to worry about freeing any memory.

Upvotes: 4

anon
anon

Reputation:

What you are writing is not C++, but C code that uses new instead of malloc, and delete instead of free. If you really want to write C++ code, start again. Read a book like Accelerated C++, which will teach you modern idiomatic C++.

Upvotes: 2

Related Questions