Assigning values to flexible array member of struct in C

Question

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
}

shape->vertices = vertices; points the pointer to a local variable which will go out of scope when the function exits. — John3136
– John3136, Commented Sep 30, 2022 at 3:48
shape->vertices = vertices; sets a pointer to the address of a local variable (array) that vanishes when the function exits... You've malloced space, but you need to fill that space... (PS: malloc can return NULL...) — user17592432
– user17592432, Commented Sep 30, 2022 at 3:49
You do not have a struct with a flexible array member. That would be Vector2D vertices[]; instead of Vector2D *vertices;. And those you would be able to set via shape->vertices[i].x and shape->vertices[i].y, but not via an initializer. — Cheatah
– Cheatah, Commented Sep 30, 2022 at 3:55

Avi Berger · Accepted Answer · 2022-09-30 05:03:48Z

There are 2 basic approaches to accomplish what you are attempting: one with and one without using a flexible array member. Your posted code is half of one way and half the other, so the pieces aren't working together.

A version of the FAM approach would be:

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

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

typedef struct Shape {
    int n;
    Vector2D vertices[];  // FAM
} Shape;

Shape *create_square(float side_length) {

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

    //one malloc to include everything
    Shape *shape = malloc(sizeof(Shape) + 4 * sizeof(Vector2D));
    shape->n = 4;
    memcpy( shape->vertices, vertices, sizeof(vertices) );

    return shape; 
}

A version of the non-FAM approach that also produces a dynamically allocated Shape (as suggested by @John Bollinger) would be:

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

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

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

Shape *create_square(float side_length) {

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

    // one malloc for the base shape and
    // a second one for the array pointed to by a member 
    Shape *shape = malloc( sizeof(Shape) ); 
    shape->n = 4;
    shape->vertices = malloc( 4 * sizeof(Vector2D));
    memcpy( shape->vertices, vertices, sizeof(vertices) );

    return shape; 
}

Another version of the non-FAM approach which doesn't produce a dynamically allocated shape (as part of your posted code appears to attempt) would be:

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

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

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

Shape create_square(float side_length) {

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

    // one malloc for the array pointed to by a member
    // shape is a local variable that can go out of scope,
    // but can also be passed or returned by value rather than via pointer 
    Shape shape; 
    shape.n = 4;
    shape.vertices = malloc( 4 * sizeof(Vector2D));
    memcpy( shape.vertices, vertices, sizeof(vertices) );

    return shape; 
}

In all cases you also would have to provide for releasing the dynamically allocated memory when you are done with it.

Other versions are possible and may have different usage characteristics.

Notable issues with using a FAM is that they must be dynamically allocated and you cannot create arrays of them. (arrays of pointers to them will work.)

This answer would be improved by addition of an example with a dynamically-allocated non-FAM structure, to better show which differences are because of the details of the data structure, and which are because of the manner of its allocation.

CodeWhore · Accepted Answer · 2022-09-30 04:10:21Z

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);

Thank you so much! This solved my problem! I am very new to C so I was not aware of the bitwise copying that would take place if I didn't return a pointer.

Collectives™ on Stack Overflow

Assigning values to flexible array member of struct in C

2 Answers 2

1 Comment

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related