Thread: free() doesn't seem to work...

  1. #1
    Registered User
    Join Date
    Jan 2008
    Posts
    3

    free() doesn't seem to work...

    I'm working on a program that maintains a log file, and one of my tasks is to manage a linked list that stores the strings that I want to log. There is a function called clearlog that is meant to free the linked list (among another purpose which I haven't coded, yet).

    The issue seems to be, however, that when I call free(), it doesn't seem to work...the pointer is still pointing to the information. I'm sure the fix is extremely simple, but I've been staring at this code for hours with no luck. Could one of you gurus help me?

    loglib.c:
    Code:
    // Preprocessor Directives
    
    #include <stdlib.h>
    #include <string.h>
    #include <errno.h>
    #include "log.h"
    
    // Log Message (Linked List) Node Structure
    typedef struct list_struct
    {
            data_t item;
            struct list_struct * next;
    } log_t;
    
    // Declarations of variables to track the head and tail of
    // the linked list.
    static log_t * headptr = NULL;
    static log_t * tailptr = NULL;
    
    // This function creates a new node and appends it at the end of
    // the message linked list.
    int addmsg( data_t data )
    {
    
            // Variable declaration
            log_t * newnode;
            int nodesize;
    
            // Allocation of space for new node and verification of action. 
            nodesize = sizeof( log_t ) + strlen( data.string ) + 1;
            if( ( newnode = ( log_t * )( malloc( nodesize ) ) ) == NULL )
                    return -1; // Returns -1 if space wasn't successfully
                               // allocated.
            
            // Copying of data from data to newnode.
            newnode->item.time = data.time;
            newnode->item.string = ( char * )newnode + sizeof( log_t );
            strcpy( newnode->item.string, data.string );
            newnode->next = NULL;
    
            // Placement of newnode in the linked list.
            if( headptr == NULL )
                    headptr = newnode;
            else
                    tailptr->next = newnode;
            tailptr = newnode;
    
            // Returns zero if function was successful.
            return 0;
    }
    
    void clearlog( void )
    {
            log_t * cnode;
            log_t * nnode;
            char * s;
    
            cnode = headptr;
    
            while( cnode != NULL ) 
            {
                    nnode = cnode;
                    cnode = cnode->next;
                    free( nnode );
                    if( nnode != NULL ) 
                            perror( "Memory failed to be freed\n" );
            }
    
            tailptr = headptr;
            
    }
    
    void test_me( void )
    {
            printf ("%s", headptr->item.string);
            return;
    }
    log.h
    Code:
    #include <time.h>
    
    typedef struct data_struct
    {
            time_t time;
            char * string;
    } data_t;
    
    int addmsg( data_t data );
    void clearlog( void );
    void test_me( void );
    driver.c
    Code:
    #include <stdio.h>
    #include "log.h"
    
    int main( int argc, char * argv[] )
    {
            data_t test;
            int return_value;
    
            test.time = time( NULL );
            test.string = "This is only a test!\n";
    
    
            if( ( return_value = addmsg( test ) ) == -1 )
            {
                    perror( strcpy( argv[0], ": Error: Memory Allocation Failed!" ) );      
                    return -1;
            }
    
            test_me();
    
            clearlog();
    
            test_me();
    
            return 0;
    }
    Output:
    This is only a test!
    Memory failed to be freed
    : Error 0
    Does anyone know what I'm doing wrong, or possibly any other causes for why free() doesn't free the allocated memory?

    I really appreciate this!

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,656
    free() doesn't set the pointer to NULL.
    It can't, the pointer is passed by value.

    > strcpy( argv[0], ": Error: Memory Allocation Failed!" )
    This is wrong, you don't know how much space has been allocated to argv[0]
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    The memory pointed to by the pointer after calling free will no longer be valid. The pointer will still point at the same location, but the memory it was pointing to is now free and unused. Thus, typically you set the pointer to NULL explicitly after calling free.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  4. #4
    Registered User
    Join Date
    Jan 2008
    Posts
    3
    If I take out the perror function, the output changes to show that, when I try to print the string stored in memory after I free that memory, it is able to, instead of giving me an error.

    Am I correct to assume that free() simply allows the heap to reallocate the memory that was originally given to me, not cleaning the memory (setting it to NULL, etc)?

    > strcpy( argv[0], ": Error: Memory Allocation Failed!" )
    This is wrong, you don't know how much space has been allocated to argv[0]
    My intention with that is to append the program name to the error message, no matter what the program's name is. I suppose I could copy the string to a different variable and then append the error message that way, since my assignment is to use perror in the program and have the error messages in the format "name: Error: detailed error message." s = strcpy(s, argv[0]), and then s = strcpy(s, ": Error: Memory Allocation Failed!") would be an appropriate fix, no?

    Thanks for all your help about free()!

  5. #5
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by AlienJedi View Post
    If I take out the perror function, the output changes to show that, when I try to print the string stored in memory after I free that memory, it is able to, instead of giving me an error.
    One thing I note now is that you're assigning a string literal ("this is a test"), and not copying it into the pointer. Therefore, it is undefined behavior to free that pointer. And it's an even greater error to modify that pointer too. So it should be const char*. Or you allocate space using malloc and use strcpy to put the string in place.

    Am I correct to assume that free() simply allows the heap to reallocate the memory that was originally given to me, not cleaning the memory (setting it to NULL, etc)?
    This is correct.

    My intention with that is to append the program name to the error message, no matter what the program's name is. I suppose I could copy the string to a different variable and then append the error message that way, since my assignment is to use perror in the program and have the error messages in the format "name: Error: detailed error message." s = strcpy(s, argv[0]), and then s = strcpy(s, ": Error: Memory Allocation Failed!") would be an appropriate fix, no?
    Strcpy copies a string into a buffer. However, it also overwrites existing contents. To append strings, you need to use strcat. Such as the rules of C.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  6. #6
    Registered User
    Join Date
    Jan 2008
    Posts
    69
    I asked this very same question the other day. After being told that free() just allows the heap to reallocate the memory space previously occupied by the variable being freed, I wrote the following code to verify:

    Code:
    int
    main(int argc, char *argv[]) {
    
      int *p = malloc(sizeof(int));
      *p = 4;
      free(p);
    
      int *q = malloc(sizeof(int));
      *q = 6;
    
      printf("p: %d\n", *p);
      printf("q: %d\n", *q);
    
      return 0;
    }
    Output:

    p: 6
    q: 6

    The memory that was originally claimed by p is taken over by q after p is freed and memory is allocated to q. Since p still points to the address on the heap currently occupied and controlled by q, when dereferenced, p gives you the value of q.

  7. #7
    Registered User
    Join Date
    Sep 2006
    Posts
    835
    Instead of printing the values of *p and *q, you could just check whether p == q, which would rule out the possibility that q pointed to a different location which held the same value by coincidence.

  8. #8
    Registered User
    Join Date
    Jan 2008
    Posts
    69
    Quote Originally Posted by robatino View Post
    Instead of printing the values of *p and *q, you could just check whether p == q, which would rule out the possibility that q pointed to a different location which held the same value by coincidence.
    Yeah, I guess that's true. Thanks for pointing that out.

  9. #9
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,613
    I'm not quite buying that yet.
    I asked this very same question the other day. After being told that free() just allows the heap to reallocate the memory space previously occupied by the variable being freed, I wrote the following code to verify:
    What you have heard is definitely true but it is not necessarily the case that you will get memory from the same address twice if you free intermittantly. Take, for example, realloc's documentation. It states that the pointer to the new memory isn't necessarily pointing to the same address as the old memory, because the memory may need to be moved elsewhere safely and then obsolete memory be freed.

    So it isn't quite that simple, and there is nothing that you could write to prove that.

    Any example with realloc in it could disprove your statement. Let's look at a data structure example:
    Code:
    #include <stdlib.h>
    #include <stdio.h>
    #include <string.h>
    #include <assert.h>
    char * readline(FILE * fsrc)
    {
        char * result = NULL;
        char buf[BUFSIZ]= "";
        unsigned long oldsize = 0;
        unsigned long newsize = 0;
    
        while( fgets(buf, BUFSIZ, fsrc) != NULL) {
            char * old;
            newsize = strlen(buf);
            if( (old = realloc(result, oldsize + newsize + 1)) != NULL)
                result = old;
            else
                return result;
            strcpy(result + oldsize, buf);
            oldsize += newsize;
    
            if(result[oldsize - 1] == '\n') {
                result[oldsize - 1] = '\0';
                break;
            }
        }
        return result;
    }
    
    int main()
    {
        char ** p;
        char ** q;
        struct person
        {
            char * name;
            struct person * next;
        };
        struct person * anyone;
    
        if( (anyone = malloc(sizeof *anyone)) != NULL) {
            printf("give this person a name:\n");
            anyone->name = readline(stdin);
            p = &anyone->name;
            anyone->next = NULL;
            
            if( (anyone->next = malloc(sizeof *(anyone->next))) != NULL) {
                printf("give another a name:\n");
                anyone->next->name = readline(stdin);
    
                printf("now change &#37;s\'s name:\n", anyone->name);
                free(anyone->name);
                anyone->name = readline(stdin);
                q = &anyone->name;
    
                /** p is the old name, q is the new one **/
                assert( *p == *q );
    
                free(anyone->next->name);
                free(anyone->next);
            }
            free(anyone->name);
            free(anyone);
        }
        return 0;
    }
    There is nothing preventing the assertion from failing if I understand myself correctly, though it may take extreme conditions for that to happen.
    Last edited by whiteflags; 01-29-2008 at 02:03 AM.

  10. #10
    Registered User
    Join Date
    Jan 2008
    Posts
    69
    Oh, absolutely. What I was trying to show was that it *can* happen, not that it's guaranteed to happen every time. At least I understand that much.. : )

  11. #11
    Registered User
    Join Date
    Jan 2008
    Posts
    3
    One thing I note now is that you're assigning a string literal ("this is a test"), and not copying it into the pointer. Therefore, it is undefined behavior to free that pointer. And it's an even greater error to modify that pointer too. So it should be const char*. Or you allocate space using malloc and use strcpy to put the string in place.
    I think I'll allocate memory using malloc and use strcpy to put the string into place.

    The main thing I wanted to see with that section of code was if my functions worked. My aim is to have me enter in what will go into the log (possibly by piping the output of another program into it.

    My other purpose is to create a log file to record the information into. My goal is to have the string entered in somehow, placed into the linked list, and when all strings are placed, everything will be outputted to a file. It's cumbersome, but the instructions are only to code four functions to facilitate a driver of some sort, so I need to fall back on creativity to define what exactly the driver will do.

    Strcpy copies a string into a buffer. However, it also overwrites existing contents. To append strings, you need to use strcat. Such as the rules of C.
    Looking back on the code, I think you're right Thanks.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. How can I free what strtok returns?
    By registering in forum C Programming
    Replies: 3
    Last Post: 06-24-2003, 04:56 PM
  2. How to free memory in this program
    By Coconut in forum C Programming
    Replies: 1
    Last Post: 10-26-2002, 10:57 PM
  3. free() and struct
    By DrakkenKorin in forum C Programming
    Replies: 2
    Last Post: 11-28-2001, 09:24 PM
  4. Free MIT courses via web.
    By Justin W in forum A Brief History of Cprogramming.com
    Replies: 4
    Last Post: 11-15-2001, 09:41 PM