I have been struggling with the ideas behind malloc and realloc for quite some time now and at the moment I have a problem with dynamically creating an array of structs. I have a struct triangle which itself is composed of an array of struct coordinates. I would like to be able to have an array of triangles which is as large as necessary, but every time I attempt to increase the length of my array, nothing seems to happen. Realloc doesn't fail and neither does malloc. However the new triangles are not inserted in my array. Here is my code for reference.
#include <stdlib.h>
#include <stdbool.h>
#include <string.h>
#include <errno.h>
#include <stdio.h>
struct coordinate {
int x;
int y;
};
struct triangle {
struct coordinate point[3];
};
static size_t size = 0;
static void addTriangle(struct triangle **triangles, struct triangle *t) {
struct triangle *ts = (struct triangle*) realloc(*triangles, (size+1) * sizeof(struct triangle));
if(ts == NULL) {
free(ts);
exit(EXIT_FAILURE);
}
*triangles = ts;
triangles[size] = t;
size++;
}
int main() {
struct triangle* triangles = (struct triangle *) malloc(sizeof(struct triangle));
if(triangles == NULL) {
free(triangles);
exit(EXIT_FAILURE);
}
for(int i = 0; i < 2; i++) {
struct coordinate *a = malloc(sizeof(struct coordinate));
a->x = 1 * i;
a->y = 2 * i;
struct coordinate *b = malloc(sizeof(struct coordinate));
b->x = 3 * i;
b->y = 4 * i;
struct coordinate *c = malloc(sizeof(struct coordinate));
c->x = 5 * i;
c->y = 6 * i;
struct triangle *t = malloc(sizeof(struct triangle));
t->point[0] = *a;
t->point[1] = *b;
t->point[2] = *c;
addTriangle(triangles, t);
}
}
I have tried every variation of this I have found, but I would rather not just blindly throw in & and * until something happens.
As-is, your program invokes undefined behavior when it passes an uninitialized *triangles to realloc: https://taas.trust-in-soft.com/tsnippet/t/9ff94de4 . You probably meant to pass &triangles when you called it in main.
Changing the call in main to addTriangle(&triangles, t);, the next issue is an out-of-bounds access inside addTriangle: https://taas.trust-in-soft.com/tsnippet/t/658228a1 . Again this may be because you have the wrong level of indirection and meant something like (*triangles)[size]
instead of triangles[size].
If I change the line triangles[size] = t; to (*triangles)[size] = *t; then there is no undefined behavior. You should check whether this program still does what you want, since it was modified: https://taas.trust-in-soft.com/tsnippet/t/8915bd2d
The final version:
#include <string.h>
#include <stdlib.h>
struct coordinate {
int x;
int y;
};
struct triangle {
struct coordinate point[3];
};
static size_t size = 0;
static void addTriangle(struct triangle **triangles, struct triangle *t) {
struct triangle *ts = (struct triangle*) realloc(*triangles, (size+1) * sizeof(struct triangle));
if(ts == NULL) {
free(ts);
exit(EXIT_FAILURE);
}
*triangles = ts;
(*triangles)[size] = *t; // a struct assignment
size++;
}
int main() {
struct triangle* triangles = (struct triangle *) malloc(sizeof(struct triangle));
if(triangles == NULL) {
free(triangles);
exit(EXIT_FAILURE);
}
for(int i = 0; i < 2; i++) {
struct coordinate *a = malloc(sizeof(struct coordinate));
a->x = 1 * i;
a->y = 2 * i;
struct coordinate *b = malloc(sizeof(struct coordinate));
b->x = 3 * i;
b->y = 4 * i;
struct coordinate *c = malloc(sizeof(struct coordinate));
c->x = 5 * i;
c->y = 6 * i;
struct triangle *t = malloc(sizeof(struct triangle));
t->point[0] = *a;
t->point[1] = *b;
t->point[2] = *c;
addTriangle(&triangles, t); /* pass the address of triangles
so that addTriangle can modify this variable's contents */
}
}
As long as you program in C, please do not cast the result of malloc. Simply write struct triangle* triangles = malloc(....
As noted by @aschepler in the comments, this program still leaks the memory blocks allocated from main. These can be freed at the end of each iteration without adding any undefined behavior: https://taas.trust-in-soft.com/tsnippet/t/a0705262 . Doing this, you may realize that t->point[0] = *a;, ... are in fact struct assignments and that it was unnecessary to allocate a separate struct coordinate in the first place: you could just fill in each struct coordinate member of the struct triangle. In addition, it was unnecessary to allocate the struct triangle in main, too: you could just use a local variable for that, since anyway the contents of the struct will be copied by the function addTriangle to the array that main's local variable triangles points to.
Also you don't need to call free(triangles) if triangles is a null pointer in main:
struct triangle* triangles = (struct triangle *) malloc(...
if(triangles == NULL) {
free(triangles);
exit(EXIT_FAILURE);
}
It is allowed to pass a null pointer to free, and this does what you would expect (it does nothing), but since you know that triangles is NULL in the then branch, simply call exit.
Handling the failure of realloc on the other hand is a subtle subject. Your program is doing it wrong, but it does not really matter because it calls exit immediately.
Storing information about the allocated array pointed by main's local variable triangles in a static file-scope variable size is not consistent. The two are so closely related that they should be in the same scope. Since you need addTriangle to be able to change size, you cannot simply move size to be a local variable of main, but you can move the local variable triangles of main to file scope, next to size. If you prefer to make size a local variable of main after all, you will need to pass its address to the function addTriangle so that the latter can update the former.