1

I'm a bit of a C newbie, so I'm still trying to get my head fully around when to worry about memory issues.

Suppose I have the following simple program:

#include <stdlib.h>

/* this returns a malloc()'d string */
char *get_str(int whichone);

int main(void)
{
    char *s;

    if ((s = get_str(0)) == NULL) {
        exit(1);
    }
    /* position 1 */

    if ((s = get_str(1)) == NULL) { /* position 2 */
        exit(2);
    }

    return 0;
}

Obviously, this simple of a program has no worries about memory. It allocates a few bytes (for all intents and purposes), and it exits provided our dear function didn't fail.

Now, suppose I am running similar code inside of a looping and fork()ing program. Should I be using free(s) at position 1 since at position 2 I leave the old value behind and assign a new location for the pointer?

2mac
  • 1,609
  • 5
  • 20
  • 35
  • in one word: yes. the same applies when you exit from the scope. – Karoly Horvath Aug 16 '14 at 12:31
  • 3
    You dont free a *pointer*, you free the memory *object* that it **points to**. BTW: your code will leak memory if the get_str() actually allocates memory. – wildplasser Aug 16 '14 at 12:31
  • You mean it will leak memory if I do not `free()` the memory being pointed to, correct? – 2mac Aug 16 '14 at 14:09
  • Correct. Because the pointer `s` is the only way to get to the object. By assigning again to `s`, it's previous content (the address of the first `getstr()` result) is overwritten. – wildplasser Aug 16 '14 at 19:49
  • Is there any OS that supports `fork()` but does not free any unfreed memory when exiting the process? – M.M Aug 16 '14 at 22:21

3 Answers3

4

Yes. free releases the memory associated with the pointer that is being free'd. Once you reassign s so that it holds a different memory location, you will have lost your opportunity to free the memory associated with the original value of s.

Alex Kleiman
  • 709
  • 5
  • 14
3

You should definetly free the memory at position 1, because once you reassigned s you lost any chance to do so. And I'd do it one way or another because you can never know when somebody else is instructed to build a fork into your application and he will assume you did everything right.

nvoigt
  • 75,013
  • 26
  • 93
  • 142
2

If you have a pointer to memory that have been allocated using one of the malloc() type calls then when you are done with the memory area, you should deallocate the memory using free(). Similarly using calloc(), which also allocates from the heap, should be followed by the use free() to deallocate the memory when done with it.

The other memory allocator such as alloca() which allocates from the stack and not the heap see On the use and abuse of alloca does not use the free() function and that memory will be recovered automatically as the stack pointers are adjusted when the function in which it is used returns. Naturally that means that using a pointer from alloca() is only good for the function and any functions it calls and the address becomes invalid as soon as the function using alloca() returns.

In your simple example, I would use free() just before the call to the get_str(1) function at position 2 so the source would look something like"

int main(void)
{
    char *s;

    if ((s = get_str(0)) == NULL) {
        exit(1);
    }

    // doing stuff with the string pointed to by s
    /* position 1 */

    // free up the string area so that we can get another one.
    free (s);
    if ((s = get_str(1)) == NULL) { /* position 2 */
        exit(2);
    }

    return 0;
}

I might also be tempted to modify your example a touch and do something like the following. This was all written without testing in a compiler so there may be a compilation error or two however this should provide the basic idea.

The idea with this approach is to have a struct that contains state information as well as a pointer so that you will know which type of get_str() pointer came from, if you are interested, and when you call the get_str() function, it will deallocate the memory for you. You could also add intelligence so that if there is memory already allocated and it is the correct type, then you do not do a free() followed by a malloc() but instead just return back.

Another bonus provided by this approach is when a free() is done, the char * member of the struct is set to NULL which should give you a crash if you try to dereference the pointer and since the whichone member of the struct indicates the last type of get_str() used, your debugging may be easier depending on how good you are with interpreting a crash dump.

#include <stdlib.h>

typedef struct {
   int   whichone;
   char  *s;
} GetStrStruct;

/* this returns a malloc()'d string */
GetStrStruct *get_str(int whichone, GetStrStruct *pStruct)
{

    free(pStruct->s);  // we depend on pStruct->s being valid or NULL here
    pStruct->s = NULL;
    pStruct->whichone = -1;
    switch (whichOne) {
    case 0:     // allocate string one
        pStruct->s =  malloc (32);   // string one type of memory allocation
        pStruct->whichone = whichone;
        break;
     case 1:     // allocate string two
        pStruct->s = malloc (64);   // string two type of memory allocation
        pStruct->whichone = whichone;
        break;
    default:
        break;
    }

    // if the malloc() failed then return a NULL pointer
    // we just reuse the pStruct pointer here, it is local and one less variable to make
    if (pStruct->s == NULL) pStruct = NULL;

    return pStruct;
}


int main(void)
{
    GetStrStruct myStruct = {0};   // create and initialize the struct

    if (get_str(0, &myStruct) == NULL) {
        exit(1);
    }

    // do things with myStruct.s, the string from last get_str() call
    // this would be using myStruct.s and not just myStruct or s so maybe awkward?
    /* position 1 */

    if (get_str(1, &myStruct) == NULL) { /* position 2 */
        exit(2);
    }

    // do things with the second type of get_str()
    // this would be using myStruct.s and not just myStruct or s so maybe awkward?

    // release the memory as I am done.  This may seem a bit strange to get a nothing string.
    get_str (-1, &myStruct);
    return 0;
}

If you added a debug facility you could even track which line of source did the last allocation and the last deallocation. Or if the malloc() fails, you could implement a debugger interrupt or other mechanism to immediately stop so you will know exactly which line the function call failed.

In the include file with the prototype for the get_str() function and the GetStrStruct struct you would use C Preprocessor as follows. If you change the 0 in the #if 0 to a 1 then the debug version is enabled otherwise it is not. The idea is to use a macro through the C Preprocessor to replace calls to get_str() with a call to get_str_Debug() and provide additional arguments with the source file path and the line number in the source file.

#if 0
        typedef struct {
           int   whichone;
           char  *s;
           struct  {
              int  lineNo;
              char  file[64];
           } myDebug;
        } GetStrStruct;

GetStrStruct *get_str_Debug(int whichone, GetStrStruct *pStruct, char *file, int line);
#define get_str(wo,ps)  get_str_Debug (wo,ps,__FILE__,__LINE__)
#else
        typedef struct {
           int   whichone;
           char  *s;
        } GetStrStruct;
GetStrStruct *get_str(int whichone, GetStrStruct *pStruct);

#endif

And then in the implementation file you could use the C Preprocessor with something like the following to allow you to specify whether the get_str() function should be replaced by a get_str_Debug() function with additional debug aids at compile time.

#if defined(get_str)
    // provide a prototype for the replacement function signature.
    GetStrStruct *get_str_Special(int whichone, GetStrStruct *pStruct);

    // provide the debug information wrapper so that we can track where the last use came from
    GetStrStruct *get_str_Debug(int whichone, GetStrStruct *pStruct, char *file, int line)
    {
        GetStrStruct *pTemp = get_str_Special (whichone, pStruct);

        if (pTemp) {
            // update the debug information.  we keep only last 60 chars of file path.
            int iLen = strlen (file);

            if (iLen > 60) iLen -= 60; else ilen = 0;
            strcpy (pStruct->myDebug.file, file + iLen);
            pStruct->myDebug.lineNo = line;
        } else {
            // cause a debugger interrupt or a crash dump or something.
        }

        return pTemp;
    }

GetStrStruct *get_str_Special (int whichone, GetStrStruct *pStruct)
#else

    /* this returns a malloc()'d string */
    GetStrStruct *get_str(int whichone, GetStrStruct *pStruct)
#endif
    {

        free(pStruct->s);
        pStruct->s = NULL;
        pStruct->whichone = -1;
        switch (whichOne) {
        case 0:     // allocate string one
            pStruct->s =  malloc (32);   // string one type of memory allocation
            pStruct->whichone = whichone;
            break;
         case 1:     // allocate string two
            pStruct->s = malloc (64);   // string two type of memory allocation
            pStruct->whichone = whichone;
            break;
       default:
            break;
        }

        // if the malloc() failed then return a NULL pointer
        if (pStruct->s == NULL) pStruct = NULL;

        return pStruct;
    }

Then where ever you use get_str() if you turn on the debugging, the C Preprocessor will substitute a call to get_str_Debug() with the arguments used in the get_str() call along with two additional arguments, a char pointer to the source file name where the get_str() is being replaced by the get_str_Debug() and the source file line number.

Community
  • 1
  • 1
Richard Chambers
  • 16,643
  • 4
  • 81
  • 106