Search code examples
cmemorystructdynamic-memory-allocationflexible-array-member

Assigning values to flexible array member of struct in C


I have a struct representing a shape in C with a flexible array member of vertices. I have a function which should return a square Shape. I have tried to do this several ways, and I keep getting incorrect values when I access the vertices' x and y properties from the flexible array member. I assume this is because I'm not allocating memory properly. What am I doing wrong?

My structs:

typedef struct Vector2D {
    float x, y;
} Vector2D;

typedef struct Shape {
    int n;
    Vector2D *vertices;
} Shape;

My function to return a square

Shape create_square(float side_length) {

    // Define vertices
    Vector2D vertices[4] = {
            {0, 0},
            {0, side_length},
            {side_length, side_length},
            {side_length, 0}
    };

    Shape *shape = malloc(sizeof(Shape) + 4 * sizeof(Vector2D));
    shape->n = 4;
    shape->vertices = vertices;

    return *shape; // Gives totally junk values
}

Alternate way to return a square

Shape create_square(float side_length) {

    // Define vertices
    Vector2D vertices[4] = {
            {0, 0},
            {0, side_length},
            {side_length, side_length},
            {side_length, 0}
    };

    return (Shape) {4, vertices}; // Gives all 0s most of the time
}

Solution

  • Not only do you have a scope issue with your verticies[4] going out of scope. Which means that the system may allocate that memory area to something else which is why it is garbage although you might get lucky (unlucky) and the actual values get passed back. You have a memory leak. You are allocating space on the memory heap for your shape but passing it back as a struct not a pointer. So what will happen is that the compiler will do a bitwise copy of your malloc'ed shape and your malloced area will forever remain allocated to your process.

    Shape create_square(float side_length) {
    
    // Define vertices
    Vector2D vertices[4] = {
            {0, 0},
            {0, side_length},
            {side_length, side_length},
            {side_length, 0}
    };
    
    Shape *shape = malloc(sizeof(Shape) + 4 * sizeof(Vector2D));
    shape->n = 4;
    shape->vertices = vertices;
    
    NOTE: You need to return shape (the pointer) so the caller can manage the 
    freeing of the shape.
    return *shape; // Gives totally junk values
    }
    
    Suggested edits:
    
    Shape *create_square(float side_length) // <- note return a pointer
    {  
        Shape *shape = (Shape*)malloc(sizeof(Shape));
        shape->n = 4;
        shape->vertices = (Vector2D *) malloc(sizeof(Vector2D));
        shape->vertices[0].x = 0.0;
        shape->vertices[0].y = 0.0;
        shape->vertices[1].x = 0.0;
        shape->vertices[1].y = side_length;
        shape->vertices[2].x = side_length;
        shape->vertices[2].y = side_length;
        shape->vertices[3].x = side_length;
        shape->vertices[3].y = 0.0;
    
        return shape; // Return ptr to a shape. 
    }
    
    DestroyShape(Shape *shape)
    {
        free(shape->verticies);
        free(shape);
    }
    
    //Caller:
    Shape *s = create_square(side_length);
    // do some stuff with it
    DestroyShape(s);