Chris Camacho
Chris Camacho

Reputation: 1174

free causing seg fault on allocated array of structs

I think I seem to be allocating space for each struct in an array (looking at the addresses of the first element of each struct in the array)

I'm obviously not understanding how C allocates stuff... using valgrind I see things like

==9852== Use of uninitialised value of size 8
==9852==    at 0x400740: main (Test.c:24)

which I'm confused by. I've looked at a number of post regarding arrays, structs, and allocation, but can't seem to see the subtlety of what's going on.

What I'm also finding puzzling is that everything works except free, for example the printing the array values shows the expected results. I could understand a seg fault when assigning to unallocated (NULL) memory or outside the size allocated (possibly) but don't understand what's going on with free

// Vector3.h

#include <stdio.h>

typedef struct {
   double    x,y,z;
} Vector3;

void Vector3Print(Vector3 v);


// Vector3.c

#include "Vector3.h"

void Vector3Print(Vector3 v) {
  printf("%f, %f, %f\n", v.x, v.y, v.z);
}


// Mesh.h

#include "Vector3.h"

typedef struct {
   Vector3  position;
   Vector3  rotation;
   Vector3* Vertices;
} Mesh;

void MeshAllocate( Mesh mesh, int size);
void MeshRelease( Mesh mesh);


// Mesh.c

#include <stdlib.h>
#include "Mesh.h"

void MeshAllocate( Mesh mesh, int size) {  // size in verts
  mesh.Vertices = malloc(size * sizeof(Vector3));
  if (mesh.Vertices==NULL) {
    printf("Error allocating memory!\n");
  }
}

void MeshRelease( Mesh mesh) {
  free(mesh.Vertices);
}


// test.c

// gcc -g -std=c99 *.c -o test

#include "Mesh.h"

int main () {

  Mesh mesh;

  printf("sizeof double %lu\n",sizeof(double));
  printf("sizeof Vector3 %lu\n",sizeof(Vector3));

  MeshAllocate(mesh,3);

  printf("address v0.x %lu\n",(unsigned long)&mesh.Vertices[0].x);
  printf("address v0.y %lu\n",(unsigned long)&mesh.Vertices[0].y);
  printf("address v0.z %lu\n",(unsigned long)&mesh.Vertices[0].z);
  printf("address v1.x %lu\n",(unsigned long)&mesh.Vertices[1].x);

  mesh.Vertices[0] = (Vector3){0.1,2.3,4.5};
  mesh.Vertices[1] = (Vector3){6.7,8.9,10.11};
  mesh.Vertices[2] = (Vector3){12.13,14.15,16.17};

  for (int i=0; i<3; i++ ) {
    Vector3Print(mesh.Vertices[i]);
  }

  MeshRelease(mesh);
}

Upvotes: 1

Views: 72

Answers (2)

Matt O
Matt O

Reputation: 1346

Your allocation and freeing looks fine to me, the problem is that you're passing in your Mesh object by value instead of by reference, which means that within the scope of MeshRelease and MeshAllocate you're dealing with new copies of the Mesh. When you go in to MeshRelease, you're attempting to free unallocated memory since the "mesh" object in that context never had it's memory allocated (it's not the same Mesh that MeshAllocate was operating on).

You can fix it by passing in the address of the Mesh to those two functions instead.

test.c

#include "Mesh.h"

int main () {

  Mesh mesh;

  printf("sizeof double %lu\n",sizeof(double));
  printf("sizeof Vector3 %lu\n",sizeof(Vector3));

  MeshAllocate(&mesh,3);

  printf("address v0.x %lu\n",(unsigned long)&mesh.Vertices[0].x);
  printf("address v0.y %lu\n",(unsigned long)&mesh.Vertices[0].y);
  printf("address v0.z %lu\n",(unsigned long)&mesh.Vertices[0].z);
  printf("address v1.x %lu\n",(unsigned long)&mesh.Vertices[1].x);

  mesh.Vertices[0] = (Vector3){0.1,2.3,4.5};
  mesh.Vertices[1] = (Vector3){6.7,8.9,10.11};
  mesh.Vertices[2] = (Vector3){12.13,14.15,16.17};

  for (int i=0; i<3; i++ ) {
    Vector3Print(mesh.Vertices[i]);
  }

  MeshRelease(&mesh);
}

Mesh.c

#include <stdlib.h>
#include "Mesh.h"

void MeshAllocate( Mesh* mesh, int size) {  // size in verts
  mesh->Vertices = malloc(size * sizeof(Vector3));
  if (mesh->Vertices==NULL) {
    printf("Error allocating memory!\n");
  }
}

void MeshRelease( Mesh* mesh) {
  free(mesh->Vertices);
}

Mesh.h

#include "Vector3.h"

typedef struct {
   Vector3  position;
   Vector3  rotation;
   Vector3* Vertices;
} Mesh;

void MeshAllocate( Mesh* mesh, int size);
void MeshRelease( Mesh* mesh);

Upvotes: 2

Russ Schultz
Russ Schultz

Reputation: 2689

void MeshAllocate( Mesh mesh, int size) {  // size in verts
  mesh.Vertices = malloc(size * sizeof(Vector3));
  if (mesh.Vertices==NULL) {
    printf("Error allocating memory!\n");
  }
}

Your error is in this function.

Your code appears to work up to free() because you get lucky.

Upvotes: -1

Related Questions