Why is there no memory leak?

Question

The following is a constant variable of char length and is printed in good format for logging. I am sure readers will have suggestions on how this can be improved, and I would welcome it.

What puzzles me is that I expected that with every call to ToHexString () it would be necessary to free () the returned static char. Instead, I do not see a memory leak. Even if I use the inline function and therefore do not assign its return value to a variable.

I created a simple test that calls this function in a loop, each time with a variable length of cString and an nMaxChars parameter. Then I looked at the VM status. The memory allocation for my test program and free memory never changed.

It seems to me that it should have increased every time malloc is called and not freed.

static char *ToHexString(const char *cString,int nMaxChars) { static char *cStr; /*if (80>strlen(cString)) nRawChars=strlen(cString); if (nMaxChars>nRawChars) nRawChars=nMaxChars; */ if (nMaxChars==0) nMaxChars=80; printf("There are %i chars\n",nMaxChars); char *cStr1; char *cStr2; char *cStr3; int nLen=nMaxChars*6; cStr=calloc(nLen,sizeof(char)); cStr1=calloc(10,sizeof(char)); cStr2=calloc(nLen,sizeof(char)); cStr3=calloc(nLen,sizeof(char)); cStr1[0]='\0'; cStr2[0]='\0'; cStr3[0]='\0'; int nC1=0; int nRowCnt=0; for (nC1=0;nC1<nMaxChars;nC1++) { ++nRowCnt; if (cString[nC1]==0x00) snprintf(cStr1,8,"[00] "); else snprintf(cStr1,8,"[%02x] ",(unsigned char)cString[nC1]); if ( (nRowCnt%8==0) ) { snprintf(cStr3,nLen,"%s%s\n",cStr2,cStr1); } else snprintf(cStr3,nLen,"%s%s",cStr2,cStr1); snprintf(cStr2,nLen,"%s",cStr3); } snprintf(cStr,nLen,"%s",cStr3); free(cStr1); free(cStr2); free(cStr3); return(cStr); } 

Here is the calling procedure:

 for (i=0;i<100;i++) { memset(&cBuff, 0,255); printf("Reading %s now..\n",cPort); while (sleep(1)==-1); nChars=read(nPort, cBuff, 255); //printf("Read %i chars from %s\n",nChars,cPort); if (nChars<=0) printf("Read 0 chars from %s\n",cPort); else printf("Read %i chars from %s\n%s\n",nChars,cPort,ToHexString(cBuff,nChars)); } 

Answer 1

The following is a leak:

 static void memeat(void) { static char *foo = NULL; foo = malloc(1024); return; } 

Valgrind output:

 ==16167== LEAK SUMMARY: ==16167== definitely lost: 4,096 bytes in 4 blocks ==16167== indirectly lost: 0 bytes in 0 blocks ==16167== possibly lost: 0 bytes in 0 blocks ==16167== still reachable: 1,024 bytes in 1 blocks ==16167== suppressed: 0 bytes in 0 blocks ==16167== Rerun with --leak-check=full to see details of leaked memory 

Note. still reachable (1024 bytes) - the result of the last occurrence of memeat() . The static pointer still retained a valid reference to the last memeat() block allocated when exiting the program. Only not the previous blocks.

The following is a leak:

 static void memeat(void) { static char *foo = NULL; foo = realloc(foo, 1024); return; } 

Valgrind Output:

 ==16244== LEAK SUMMARY: ==16244== definitely lost: 0 bytes in 0 blocks ==16244== indirectly lost: 0 bytes in 0 blocks ==16244== possibly lost: 0 bytes in 0 blocks ==16244== still reachable: 1,024 bytes in 1 blocks ==16244== suppressed: 0 bytes in 0 blocks ==16244== Rerun with --leak-check=full to see details of leaked memory 

Here, the specified address foo was freed, and foo now points to the newly allocated address and will continue to do so next time memeat() .

Explanation:

The static storage type says that the pointer foo will point to the same address that is initialized each time the function is entered. However, if you change this address every time a function is entered via malloc() or calloc() , you have lost the link to the blocks from the previous distribution. Consequently, a leak, as it is either going to return a new address.

'Still Reachable' in valgrind means that all allocated heap blocks still have a valid pointer to access / manipulate / free them when exiting. This is similar to allocating memory in main() and not freeing it, just relying on the OS to recover the memory.

In short, yes - you have a leak. However, you can fix this quite easily. Just keep in mind that you really rely on your OS to recover memory unless you add another argument to your function that simply tells ToHexString to call the static pointer for free, which you could use when exiting.

Similar to this: (full test program)

 #include <stdlib.h> static void memeat(unsigned int dofree) { static char *foo = NULL; if (dofree == 1 && foo != NULL) { free(foo); return; } foo = realloc(foo, 1024); return; } int main(void) { unsigned int i; for (i = 0; i < 5; i ++) memeat(0); memeat(1); return 0; } 

Valgrind Output:

 ==16285== HEAP SUMMARY: ==16285== in use at exit: 0 bytes in 0 blocks ==16285== total heap usage: 6 allocs, 6 frees, 6,144 bytes allocated ==16285== ==16285== All heap blocks were freed -- no leaks are possible 

Note on final output :

Yes, in fact, 6144 bytes were allocated according to the fact that malloc() was returned at program startup, but it just means that the static pointer was freed and then redistributed according to the number of times memeat() . The actual use of the program heap at any given time was actually only 2 * 1024, 1k to highlight the new pointer, while the old one still existed, waiting to be copied to the new one.

Again, it should not be too difficult to configure your code, but it is not clear to me why you are using static storage to get started.

Answer 2

This is a memory leak. If you constantly call a function, the memory used by the program increases. For instance:

 int main() { while (1) { ToHexString("testing the function", 20); } } 

If you run this and watch the process using the system monitoring tool, you will see that the memory used is constantly increasing.

The leak is probably not obvious in your program, because the function only loses a few bytes of each call and is not called very often. Thus, the increase in memory usage is not very noticeable.

Answer 3

I would like to point out two things that are striking when checking the code:

  cStr = calloc (nLen, sizeof (char));

Why didn’t you perform error checking on this ... as I see from the code, there is a zero check in the assumptions memory. It will always be available .... dangerous .... ALWAYS check for a NULL pointer to return from a memory allocation function call, for example calloc , malloc and realloc ... it will ALWAYS be on a programmer to control free clicking pointers to return them back to the pile.

Also, since you have cStr declared as a static pointer to char * , you don't free it at all, this line proves this:

  printf ("Read% i chars from% s \ n% s \ n", nChars, cPort, ToHexString (cBuff, nChars));
                                                   ^^^^^^^^^^^^

You would be better off doing it like this:

  char * hexedString;
 hexedString = ToHexString (cBuff, nChars);
 ....
 printf ("Read% i chars from% s \ n% s \ n", nChars, cPort, hexedString);
 ....
 free (hexedString);

Answer 4

You are returning the result of your procedure in a new array. With your model, the responsibility for freeing this array with the result is in the caller. Thus, there, in the caller, you must store the result of your routine in a temporary mode, do whatever you want with it, and then release () at the end.