Leaking Memory

[made_of_asp]

Is this function leaking memory?


char** VirtualSplitString(char* pString, char sep, int& uBound)
{
unsigned long s = 0; unsigned long i = 0;
char** pArray = (char**)calloc(0, sizeof(char));

//allocate empty storage

//the first array
pArray[0] = (char*)calloc(1024, sizeof(char));

//separate an array...
while(*pString)
{
//go through the string, find position
if(*pString == sep)
{
i++;
s = 0; //current position
//allocate space for new input
pArray[i] = (char*)calloc(1024, sizeof(char));
}
else
{
//get the current character and set it
pArray[i][s] = *pString;

//increase character position
s++;
}

//increase string position
pString++;

}

uBound = ++i;

//return the pArray data type
return pArray;
};


thanks for any help...

[jim mcnamara]

Yes. For each calloc() call you should call free().

Actually, it could be quite large, quite fast because you are allocating 1024 bytes repeatedly.

[CornedBee]

And the first calloc call is wrong.
a) You must allocate char*, not char.
b) You must allocate more than 0 items. Actually you must allocate one item per substring in pString. You either have to reallocate when the buffer is too small or you traverse the string once and check how many substrings there are, then allocate, then traverse it a second time.

Of course if you free() the memory you allocated you can't return it anymore.
The user must pass an array that is large enough, which you just fill.

[made_of_asp]

Thanks for your replies :)

i see the point, so can i do something like this


char** p;

p = VirtualSplit(...

for(int i = 0; i < bound; i++)
free(p[i]);


would that work?

sorry i am new to these double pointers so i get confused fast. and these memory leaks are fairly bad because i am writing an FTP server... no good if it drains memory.

[CornedBee]

After you've freed all p[i] you also need to free p itself.

It basically works, but it is bad manner to write it this way. It is better if the caller of the function allocates memory.

[jim mcnamara]

What CB means - if a function allocates memory, it frees memory it allocated also. This isn't so much that it's 'bad form', whichit is.

Without doing that you can't easily find memory leaks or other problems - especially six months later when you have totally forgotten your code.

The only other 'out' is to use global variables - ones defined outside of a function. Most professional db coders (Oracle, etc)
tend to do this because it reduces the total number of lines and allows debugging any module in the presence of most of the important variables. It has serious downsides as well. Like huge loadable image size, excessive demand-zero faulting on load....

:rolleyes:

[CornedBee]

... name conflicts, thread sync problems...