views:

56

answers:

1

I have this code:

void getStringWithTag(char* pFile, long sizeSound, char* tag, char* ret, float* currentPos){
char* tagCopy;
tagCopy = (char*)malloc(256 * sizeof(char));
int totalLen = 0;
int tempLen = 0;
memset(tagCopy, 0, 256);
memset(ret, 0, 128);
pFile += ((int)*currentPos);
while (totalLen+(*currentPos) < sizeSound) {
    if (*pFile == '<') {
        pFile += 5;
        totalLen += 5;
        while(*pFile != '>'){
            *tagCopy = *pFile;
            tagCopy++;
            pFile++;
            totalLen++;
            tempLen++;
        }
        tagCopy -= tempLen;
        tempLen = 0;
        if (strcmp(tagCopy, tag) == 0) {
            pFile++;
            totalLen++;
            while (*pFile != '<') {
                *ret = *pFile;
                ret++;
                pFile++;
                totalLen++;
                tempLen++;
            }
            ret -= tempLen;
            *currentPos += totalLen;
            tagCopy = NULL;
            free(tagCopy);
            return;
        }else{
            memset(tagCopy, 0, 256);
        }
    }
    pFile++;
    totalLen++;
}
tagCopy = NULL;
free(tagCopy);
*currentPos = sizeSound;
ret = NULL;
}

That apparently gives me a memory leak with "tagCopy". Can anyone tell me why? I thought I had this down pretty good but this is the only place where I get a memory leak.

Thank you.

+4  A: 

You modify tagCopy a few times in that routine, and then you try to free it later. Probably dangerous. You're also setting tagCopy to NULL before calling free(), so every time you try to free NULL rather than the actual memory you allocated.

Carl Norum
hmm, do you have any suggestions on how to fix it?
funckymonk
Well said. It would be better to use array syntax (`tagCopy[i]`) or an alias pointer. Modifying `tagCopy` makes it harder to analyze.
Matthew Flaschen
@funckymonk, Matthew has good suggestions. I would say 1) don't bother setting it to `NULL` ever, and 2) avoid ever modifying it in the routine (or save a copy of the original pointer for freeing later if you prefer).
Carl Norum
@Carl Norum - Its a good practice to set a pointer to NULL after free. Isnt it?
Praveen S
@Praveen, maybe if it were a global. In this case there is absolutely no reason to do so.
Carl Norum
... in particular not *before* freeing it.
Jens Gustedt
Thank you everybody for the help!I got it to work leak free.I took Matthews suggestions and made a tagCopy[256] and used that instead.
funckymonk