user2529913
user2529913

Reputation: 1

C free an instance, which is pointed by other pointers

I have the following C code. I have two pointers pointing to the same object. It says double free error. Can someone help to see what the problem is? Thanks.

#include <stdlib.h>
#include <stdio.h>

typedef struct edge {
    int head;
} edge_t;

typedef struct edge_list_t {
    edge_t *edge;
} edge_list_t;
int main() {
    edge_list_t *p1;
    edge_list_t *p2;
    edge_t *c;

    p1 = malloc(sizeof(edge_list_t));
    p2 = malloc(sizeof(edge_list_t));

    c = malloc(sizeof(edge_t));

    p1->edge = c;
    p2->edge = c;

    free(c);

    if (p2->edge) {
        printf("not freed\n");
        free(p2->edge);
    } else {c
        printf("freed\n");
    }
    return 1;
}

Upvotes: 0

Views: 106

Answers (3)

Jonathan Leffler
Jonathan Leffler

Reputation: 755094

You shouldn't free c once you've transferred control of it to p1, and you should not have both p1 and p2 sharing a single edge pointer, and you should release p1 and p2 via a function that also releases the edge pointer. These observations lead to:

#include <stdio.h>
#include <stdlib.h>

typedef struct edge
{
    int head;
} edge_t;

typedef struct edge_list_t
{
    edge_t *edge;
} edge_list_t;

// Will acquire a loop when you have an actual list of edges
static void free_edge_list(edge_list_t *e)
{
    free(e->edge);
    free(e);
}

int main(void)
{
    edge_list_t *p1;
    edge_list_t *p2;
    edge_t *c;

    p1 = malloc(sizeof(edge_list_t));
    p2 = malloc(sizeof(edge_list_t));

    c = malloc(sizeof(edge_t));

    p1->edge = c;

    c = malloc(sizeof(edge_t));
    p2->edge = c;

    free_edge_list(p1);
    free_edge_list(p2);

    return 0;
}

In general, you should check that memory allocations succeed; this code (still) doesn't do that.

Upvotes: 0

user1944441
user1944441

Reputation:

Here:

p2->edge = c;

free(c);

when you free c the value of c does not change and even if it did the value of p2->edge would stay the same. It would hold the original value of c of course. So you always free both c and p2->edge which both hold the same value.

To avoid that set c to NULL if you called free() on it and later check if(c) ,which will return false and not free c again.

Note: free() does not change the pointer in any way. It trust you that the pointer points to correct memory that was before never free()d.

Upvotes: 1

ouah
ouah

Reputation: 145919

p2->edge = c;

free(c);

if (p2->edge) {
    printf("not freed\n");
    free(p2->edge);

^ the last free is a double free. Remember that after the first free call, the value of c is an invalid value.

Upvotes: 1

Related Questions