user1316762
user1316762

Reputation: 11

Another malloc/free dilemma

I haven't used C or C++ in over 6 years and am a bit rusty. I am writing some quick test code for a graph-traversal algorithms. The code accepts an adjacency-list style input. However I'm running into some issues with free/malloc.

There are two issues with my code:

  1. When I run the code without free and without the getchar the code hangs when I use VC++ cntrl-f5. This is remedied when I use getchar(). Does anyone know why?

  2. When I run the code with free the code hangs. I've tried to debug the code and it hangs exactly at the free statement. Any suggestions as to how I can fix this?

Also please let me know if I'm doing anything dangerous with this code. Header file is omitted.

  void * s_malloc(size_t size){
    void * ret_pntr = malloc(sizeof(size));
    if (ret_pntr == NULL){
        printf ("error");
        exit(1);
    }
    return (void *)malloc(sizeof(size));
  }

  void initialize_graph(graph * G1, int num_vertices){
    int i = 0 ;
    G1->num_vertices = num_vertices;
    G1->node_list = (node**)s_malloc(sizeof(node*)*num_vertices);
    for (i = 0; i < num_vertices; i ++){
        G1->node_list[i] = (node *)s_malloc(sizeof(node));
    }
  }

  void free_everything(graph * G1){
    int i = 0;
    node * ref = NULL;
    for (i = 0; i < G1->num_vertices; i++){
        ref = G1->node_list[i];
        recursive_remove(ref);
    }
    free(G1->node_list);
  }

  void recursive_remove(node * ref){
    if (ref == NULL){
        return;
    }
    else{
        recursive_remove(ref->next);
    }
    free(ref);
  }

  int main(){
    int i = 0;
    graph * G1 = (graph*)s_malloc(sizeof(graph));
    G1->init = &initialize_graph;
    G1->init(G1, 10);
    G1->remove = &free_everything;
    G1->node_list[0]->value = 1;
    G1->node_list[0]->next = (node*)s_malloc(sizeof(node));
    G1->node_list[0]->next->value = 2;
    G1->node_list[0]->next->next = NULL;
    G1->node_list[1]->value = 10;
    printf("%d\n", G1->node_list[0]->next->value);
    printf("%d\n", G1->node_list[1]->value);
    G1->remove(G1);
    free(G1);
    getchar();
   }

Upvotes: 0

Views: 237

Answers (1)

George Skoptsov
George Skoptsov

Reputation: 3971

One thing that jumps out immediately is that in

void * s_malloc(size_t size){
  void * ret_pntr = malloc(sizeof(size));
  if (ret_pntr == NULL){
    printf ("error");
    exit(1);
  }
  return (void *)malloc(sizeof(size));
}

you are allocating twice, leaking the first allocation, and are not checking result of the second allocation. Another is that your malloc call should be

 malloc(size)

not

 malloc(sizeof(size))

Because in your current code you underallocate all your memory (each allocation will only give you 4 bytes at a time), your accesses stomp all over... I'm surprised execution actually makes it to the getchar() or free().

What's not clear is why you are trying to emulate OOP in C while using VC++. If you rewrite this in C++ using STL containers to hold your nodes and with indices instead of pointers, I think a lot of your problems will disappear. But right now debugging this mess for you is not going to be fun for any one.

An even better solution is to use an existing graph library like Boost Graph

Upvotes: 4

Related Questions