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:

Quote:

---

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!

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]

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.

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)?

Quote:

---

> 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()!

Quote:

---

Originally Posted by AlienJedi

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.

Quote:

---

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.

Quote:

---

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.

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.

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.

Quote:

---

Originally Posted by robatino

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.

I'm not quite buying that yet.

Quote:

---

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.

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.. : )

Quote:

---

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.

Quote:

---

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.