-1

I need to have a dynamic array so I have used malloc in my code...However I don't know how to successfully free the memory afterwards. Somewhere in my code I believe I have a pointer re-assignment which leads to dangling pointer error (when i do child2=child1). Does anyone know how to free my mallocs properly? Thanks in advance.

My actual code is below:

typedef struct Edge//per solution
{
int label;//label
float weight;//energy of each edge
} edge;

// creating the chrom structure
typedef struct Chrom
{
edge **gene;
float fitness_score;
}     

In one of my functions i have the following, where pop_size and num_nodes was previously calculated as 100 and 10 respectively.

Chrom* child1;
Chrom* child2;

//allocate memory of child
child1 = malloc(num_nodes * sizeof(child1));
child2 = malloc(num_nodes * sizeof(child2));
if(child1 == NULL||child2 == NULL)
    printf("ERROR1: Memory allocation failed!");
for(x = 1; x <= num_nodes; x++)
{
    child1[x].gene = malloc(num_nodes * sizeof(edge*));
    child2[x].gene = malloc(num_nodes * sizeof(edge*));
    if(child1[x].gene == NULL||child2[x].gene == NULL)
        printf("ERROR2: Memory allocation failed!");
    for(y = 0; y < num_nodes; y++)
    {
        child1[x].gene[y] = malloc(num_nodes * sizeof(edge));
        child2[x].gene[y] = malloc(num_nodes * sizeof(edge));
        if(child1[x].gene[y] == NULL||child2[x].gene[y] == NULL)
            printf("ERROR3: Memory allocation failed!");
    }
}

//do something...

for(i=0; i<pop_size; i++)
    for(x=0; x<num_nodes; x++)
        for(y=0;y<num_nodes;y++)
            child2[i].gene[x][y].label=child1[i].gene[x][y].label;

free(child1);//can i free the memory like this?
free (child2);// will it automatically do all 'arrays'?

Also, must I first check if memory was allocated properly before freeing it?

J.DOLE
  • 9
  • 6
  • 3
    You should probably worry about the undefined behavior due to out-of-bounds access before you worry about `free()`. – EOF Jul 27 '16 at 20:19
  • 1
    Specifically `for(x = 1; x <= num_nodes; x++)` you don't seem to be aware that arrays are indexed from `0` to `len-1`. Every other instance too. – Weather Vane Jul 27 '16 at 20:21
  • 1
    Count how many `malloc()` do you use, that's how many `free()` you need. In reverse order. – Bob__ Jul 27 '16 at 20:22
  • 2
    Try `child1 = malloc(num_nodes * sizeof *child1);` – Weather Vane Jul 27 '16 at 20:25
  • 1
    _"...When I do child2=child1..."_ you are probably doing a [shallow copy](http://stackoverflow.com/a/15278367/4944425) while you (probably) need a deep copy. – Bob__ Jul 27 '16 at 20:42

2 Answers2

1
child1 = malloc(num_nodes * sizeof(child1));

this is incorrect. You are allocating space for num_nodes pointers (child1 is a pointer to Chrom). You want to allocate space for num_nodes Chrom instances. Change it to

child1 = malloc(num_nodes * sizeof(*child1));
FredK
  • 4,094
  • 1
  • 9
  • 11
0

First of all, you allocate space for Chrom pointers, not the space for Chrom structures so I am surprised that child1[x].gene works without crashing but to only answer the questions posed as comments in your code,

free(child1);//can i free the memory like this?
free (child2);// will it automatically do all 'arrays'?

child1 is an array of pointers and each of those pointers points to allocated memory which will be lost when you free(child1). I would free each pointer child1[x].gene first and then free child1. Same thing for child2.

This is probably close to what you want:

typedef struct Edge//per solution
{
  int label;//label
  float weight;//energy of each edge
} edge;

// creating the chrom structure
typedef struct Chrom
{
  edge *gene;  // changed from edge**
  float fitness_score;
};

int main(void)
{
  int num_nodes = 3;
  int x;

  struct Chrom* child1;

  // if you want num_nodes Chrom entries

  child1 = malloc(num_nodes * sizeof(struct Chrom));

  // Allocating individual edges (I don't know why you declare edge** gene
  // so I will assume that what you intended was edge* gene

  for(x = 1; x <= num_nodes; x++)
  {
     child1[x].gene = (edge*)malloc(sizeof(struct Edge));
  }


  // deallocate your memory

  for(x = 1; x <= num_nodes; x++)
  {
     free(child1[x].gene);
  }

  // free your array of Chroms

  free(child1);

  return 0;

}

Here is what the code could be if you want a 2D array of edegs within each Chrom; Also, there is a bug in my previous answer; x should be initialized to zero in the for loop rather than to 1 because this will cause an array index out of bounds and use lower-than instead of lower-than-or-equal. (WARNING: I only tested it slightly):

typedef struct Edge//per solution
{
  int label;//label
  float weight;//energy of each edge
} edge;

// creating the chrom structure
typedef struct Chrom
{
  edge **gene;  
  float fitness_score;
};

int main(void)
{
  int num_nodes = 3;
  int num_edges_x = 2;
  int num_edges_y = 3;
  int x, j;

  struct Chrom* child1;

  // if you want num_nodes Chrom entries

  child1 = malloc(num_nodes * sizeof(struct Chrom));

  // Allocating 2D array of edges for each Chrom
  // USE zero-based indexing.

  for(x=0; x < num_nodes; x++)
  {
     child1[x].gene = (edge**)malloc(num_edges_x * sizeof(edge*));

     // initialise you array of edges

     for (j=0; j<num_edges_x; j++)
     {
         child1[x].gene[j] = (edge*)malloc(num_edges_y * sizeof(edge));           
     }
  }

  // Use a child1[x].gene[x][y]

  child1[0].gene[0][0].label = 3;
  child1[0].gene[0][0].weight = 7.2F;

  printf("\nlabel: %d - weight: %f", child1[0].gene[0][0].label,   child1[0].gene[0][0].weight);

  child1[1].gene[0][0].label = 1;
  child1[1].gene[0][0].weight = 12.4F;

  printf("\nlabel: %d - weight: %f", child1[1].gene[0][0].label,  child1[1].gene[0][0].weight);

  child1[1].gene[1][0].label = 5;
  child1[1].gene[1][0].weight = 112.6F;

  printf("\nlabel: %d - weight: %f", child1[1].gene[1][0].label,      child1[1].gene[1][0].weight);

  // deallocate your memory
  for(x =0; x < num_nodes; x++)
  {

     for (j=0; j<num_edges_x; j++)
     {
        free(child1[x].gene[j]);
     }

     free(child1[x].gene);
  }

  free(child1);

  return 0;
}
clarasoft-it
  • 201
  • 1
  • 5
  • yes it is working. You mentioned creating a space for Chrom structures, how can i go about doing that to provide space for child1[x].gene? – J.DOLE Jul 27 '16 at 20:59
  • The space for every child1[x].gene structure was done in the for loop after allocating the Chrom structures. – clarasoft-it Jul 27 '16 at 21:18
  • I didnt make a mistake in declaring edge** gene. I wanted to be able to call child1[x].gene[i][j].label for example. Would I need to change it to: for(x = 1; x <= num_nodes; x++){child1[x].gene = (edge*)malloc(sizeof(struct Edge*)) } for(x = 1; x <= num_nodes; x++) for(y=1;y<=num_nodes;y++){child1[x].gene[y] = (edge*)malloc(sizeof(struct Edge)) }? – J.DOLE Jul 27 '16 at 21:40
  • Just so I understand ... within each Chrom, you want an array of edges? – clarasoft-it Jul 27 '16 at 21:52