How can I free memory from my structure containing an array of dynamically created strings?

Question

I have a struct that must contain, among other things, a dynamically created string and an array of also dynamically-created strings. However, I am finding it difficult to free this struct afterwards, as it gives me runtime errors.

So the questions are:

So far I cannot even release the string. Why?

How to create a dynamic "array" of strings inside this struct?

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

typedef struct { 
  unsigned int num;
  unsigned int* sizes;
  unsigned int* coords;
  char* name;
} TOPOLOGY;

TOPOLOGY * constructor() {
  char * name="Hardware Topology";
  TOPOLOGY * top=calloc(1,sizeof(TOPOLOGY *));
  top->num=0;
  top->name=calloc(1,strlen(name));
  strcpy(top->name,name);
  return top;
}

void destructor(TOPOLOGY * top) {
  if(top->sizes!=NULL) { free(top->sizes); }
  if(top->coords!=NULL) { free(top->coords); }
  if(top->name!=NULL) { free(top->name); } exit(0);
  if(top!=NULL) { free(top); }
}

void add(TOPOLOGY * top, unsigned int size) {
  top->num++;
  size_t s=top->num*sizeof(unsigned int*);
  top->sizes=realloc(top->sizes,s);
  top->coords=realloc(top->coords,s);
  top->sizes[top->num-1]=size;
}

void coords(TOPOLOGY * top, unsigned int coords[]) {
  int i;
  for(i=0;i<top->num;i++) {
    top->coords[i]=coords[i];
  }
}

void get(TOPOLOGY * top) {
  int i;
  for(i=0; i<top->num;i++) {
    printf("Dim: %d: %d\n",i,top->sizes[i]);
  }
}

void getcoords(TOPOLOGY * top) {
  int i;
  for(i=0;i<top->num;i++) {
    printf("Coord %d = %d\n",i,top->coords[i]);
  }
}

void setname(TOPOLOGY * top, char * name) {
  int s=sizeof(name);
  top->name=realloc(top->name,s);
  strcpy(top->name,name);
}


int main() {
  unsigned int c[4]={3,2,0,1};
  TOPOLOGY * top=constructor();
  add(top,1025);
  add(top,512);
  add(top,10);
  add(top,24);
  coords(top,c);
  get(top);
  getcoords(top);
  destructor(top);
  return 0;
}

Answer 1

First up, the constructor is wrong. It will give you the size of a pointer on your machine, so pretty much everything you do with top from then on will be illegal.

TOPOLOGY * top=calloc(1,sizeof(TOPOLOGY *)); /* Wrong. */
                                        ^
top = calloc(1, sizeof(TOPOLOGY)); /* Better. */
top = calloc(1, sizeof(*top)); /* Even better. */

Second

Everywhere you calloc strings, you must use strlen(...) + 1.

top->name=calloc(1, strlen(name) + 1);
strcpy(top->name, name);

Or, slightly laterally, use strdup:

top->name = strdup(name); /* Does malloc and whatnot for you. */

Third

Don't check for NULL when freeing. It's perfectly legal to free(NULL).

void destructor(TOPOLOGY * top)
{
    free(top->sizes);
    free(top->coords);
    free(top->name);
    free(top);
}