There is no need to cast the return of malloc, it is unnecessary. See: Do I cast the result of malloc?. malloc returns void * compatible with any pointer. A cast can only serve to muddy the waters. Simply allocate your pointer points and then call sizeof on the dereferenced pointer, e.g.:
(*arr)[i].points = malloc (sizeof *(*arr)[i].points);
While a correct statement for the allocation, it wholly fails to validate the allocation succeeds, instead, check the return from malloc is not NULL, e.g.
if (!((*arr)[i].points = malloc (sizeof *(*arr)[i].points))) {
perror ("malloc failed - points");
exit (EXIT_FAILURE);
}
The same for the allocation of *arr, e.g.
if (!(*arr = malloc (sizeof **arr * nclusters))) {
perror ("malloc failed - nclusters");
exit (EXIT_FAILURE);
}
(note: how_many_clusters was shortened to nclusters, we are not writing a novel)
Your primary problem is:
(*arr)[i].points[i] = point;
You only allocate memory for one Point struct, therefore points[i] makes no sense at all. You allocated sizeof(Point) -- how many of them do you think there are?
Since your goal is to copy point to the newly allocated block of memory at the address held by the pointer (*arr)[i].points, "How Do You Access (or Set) the value Referenced (e.g. Pointed-to) by a Pointer?" (Answer:, you dereference the pointer to obtain the value).
That's all you need do here. No additional [..] tacked on the end to dereference (*arr)[i].points, just dereference it with the unary '*', e.g.
*(*arr)[i].points = point; /* copy struct to allocated memory */
Putting the pieces altogether, you can do something like the following:
#include <stdio.h>
#include <stdlib.h>
typedef struct {
int ID; //each point has some ID and XY coordinates
int X;
int Y;
} Point;
typedef struct {
int size; //how many points can cluster hold
int count; //how many points cluster actually holds
Point *points; //dynamically allocated array of points
} Cluster;
void fill_array (Cluster **arr, int nclusters)
{
if (!(*arr = malloc (sizeof **arr * nclusters))) {
perror ("malloc failed - nclusters");
exit (EXIT_FAILURE);
}
for (int i = 0; i < nclusters; i++)
{
Point point;
point.ID = i;
point.X = i * 2;
point.Y = i * 3;
(*arr)[i].size = 1;
(*arr)[i].count = 1;
if (!((*arr)[i].points = malloc (sizeof *(*arr)[i].points))) {
perror ("malloc failed - points");
exit (EXIT_FAILURE);
}
*(*arr)[i].points = point; /* copy struct to allocated memory */
}
}
int main (void) {
Cluster *clusters;
int clusters_count = 20;
fill_array (&clusters, clusters_count);
for(int i = 0; i < clusters_count; i++)
{
free(clusters[i].points); //crash point
clusters[i].points = NULL;
}
free (clusters);
return 0;
}
(note: you do not have to free each clusters[i].points in reverse order. (it's not wrong, there is just no need to go out of your way to do it). Just free them as you work your way through the loop.)
Example Use/Validation
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to insure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ valgrind ./bin/fillclusters_alloc
==26217== Memcheck, a memory error detector
==26217== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==26217== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==26217== Command: ./bin/fillclusters_alloc
==26217==
==26217==
==26217== HEAP SUMMARY:
==26217== in use at exit: 0 bytes in 0 blocks
==26217== total heap usage: 21 allocs, 21 frees, 560 bytes allocated
==26217==
==26217== All heap blocks were freed -- no leaks are possible
==26217==
==26217== For counts of detected and suppressed errors, rerun with: -v
==26217== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
Look things over and let me know if you have further questions.
