Wednesday 20 October 2010

Memory Allocation for char*

One common problem that I have noticed is that people copy char* pointers thinking that the pointers will be valid when used but this is rarely the case. See the example below.


//Program tested on Microsoft Visual Studio 2008 - Zahid Ghadialy
//Example demonstrates allocation of memory for char*
#include<iostream>

using namespace
std;

typedef struct
{
char
const * name;
}
TypeInfo;

TypeInfo* MakeType(const char *typeName)
{

TypeInfo* newNamedTypeInfo = new TypeInfo;

char
* newTypeName = new char[strlen(typeName) + 1];
strcpy(newTypeName, typeName);
newNamedTypeInfo->name = newTypeName;

return
newNamedTypeInfo;
}


int
main()
{

TypeInfo* testType = MakeType("Apple Banana");
cout << "testType->name = " << testType->name << endl;

return
0;
}





Please suggest any improvements.

The output is as follows:
Note that there is a memory leak in the program if you can spot it and fix it then you are already half way through :)

7 comments:

  1. The conversion of char* to const char* is valid. So I don't see any obstructions there. What you don't do is you don't free allocated memory. Neither pointer to TypeInfo nor pointer to char. But I guess it is not what you wanted to emphasize. Give a clue.

    --rha

    ReplyDelete
  2. Yes thats right, I dont release the allocated memory because I use struct and havent defined any destructors.

    I actually meant that if you use this program remember to free the memory. Didnt intend someone to comment on the post and point out the actual problem :)

    ReplyDelete
  3. Hi Zahid,

    I think memory leak is there only for the newTypeName variable not the structure. Your comment above is not clear to me. The structure is any how still pointed to by testType.
    Any response is awaited.

    ReplyDelete
  4. Hello Sailesh, good to hear from you.

    What I mean is, if you add the following line before the return in main:

    delete testType;

    Will you see memory leak?

    Yes you will because the name in TypeInfo is not freed. So we can change the TypeInfo as follows:

    struct TypeInfo{
    ~TypeInfo()
    {
    if(name)
    {
    delete[] name;
    name = NULL;
    }
    }
    char const * name;
    };

    and memory leak should be gone.

    Ps: The above code will look a bit messy because blogger does not allow preformatting in comments

    ReplyDelete
  5. Yaa correct. Thats what I was trying to infer.
    Thanks a lot Zahid :)

    ReplyDelete
  6. In the introductory comments to this post, you say that you have noticed people frequently copying char pointers incorrectly. What is this incorrect way? Is this an example of what you mean?


    typedef struct {
    char const * name;
    } TypeInfo;

    TypeInfo* MakeType(const char *typeName)
    {
    TypeInfo* newNamedTypeInfo = new TypeInfo;
    newNamedTypeInfo->name = typeName;
    return newNamedTypeInfo;
    }

    int main()
    {
    TypeInfo* testType = MakeType("Apple Banana");
    cout << "testType->name = " << testType->name << endl;
    }

    //end main.cpp

    Thanks in advance for the clarification.

    ReplyDelete
  7. Hi Justin, yes you are correct. We have spent countless hours debugging why a log becomes garbage only to find out that this is the way the log string was created.

    Other problem being that people use reference to copy a local string that becomes invalid as soon as out of scope.

    ReplyDelete