Assigning values to flexible array member of struct in C-CodePudding

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
}

CodePudding user response：

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

CodePudding user response：

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