tags:

views:

531

answers:

6

Hi I am trying to use C to implement a simple struct:
2 boxes, each contains different number of particles; the exact number of particles are passed in main().

I wrote the following code:

typedef struct Particle{
    float x;
    float y;
    float vx;
    float vy;
}Particle;

typedef struct Box{
    Particle p[];
}Box;

void make_box(Box *box, int number_of_particles);

int main(){
    Box b1, b2;
    make_box(&b1, 5);  //create a box containing 5 particles
    make_box(&b2, 10); //create a box containing 10 particles
}

I've tried to implement make_box with the following code

void make_box(struct Box *box, int no_of_particles){
    Particle po[no_of_particles];
    po[0].x = 1;
    po[1].x = 2;
    //so on and so forth...
    box->p = po;
}

It's always giving me "invalid use of flexible array member". Greatly appreciated if someone can shed some light on this.

A: 

You cannot assign to Particle p[], just use Particle* p instead.

And of course you need to allocate your Particle array on heap, not on stack! Otherwise it will be destroyed as soon as you leave the function make_box.

void make_box(struct Box *box, int no_of_particles){
    Particle* po = (Particle*)malloc(sizeof(Particle)*no_of_particles);
    for (int i = 0; i < no_of_particles; i++)
        po[0].x = i;
    //so on and so forth...
    box->p = po;
}

and

struct Box
{
    Particle* p;
};

And don't forget to deallocate the memory when you don't need it any more.

Vlad
A: 

in Box struct, why don't you use a pointer to a Particle so you can create a dynamic array?

Taz
+2  A: 
void make_box(struct Box *box, int no_of_particles){
    Particle po[no_of_particles];
    //...
    box->p = po;
}

po is a local variable, and the storage for it is allocated automatically on the stack; returning its address is a bad idea. You should instead allocate memory from the heap, and remember freeing that memory when you're done with the boxes:

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

typedef struct Particle_ {
    float x;
    float y;
    float vx;
    float vy;
} Particle;

typedef struct Box_ {
    Particle *p;
} Box;

void make_box(Box *box, int no_of_particles);

void make_box(Box *box, int no_of_particles){
    Particle *po = (Particle *) malloc ( no_of_particles*sizeof(Particle) );
    po[0].x = 1;
    po[1].y = 2;
    //so on and so forth...
    box->p = po;
}

void destroy_box(Box *box){
    free(box->p);
}


int main(){
    Box b1, b2;
    make_box(&b1, 5);  //create a box containing 5 particles
    make_box(&b2, 10); //create a box containing 10 particles

    // do the job...
    printf("box b1, point 0, x: %5.2f\n", b1.p[0].x);
    printf("box b2, point 1, y: %5.2f\n", b2.p[1].y);

    destroy_box(&b1);
    destroy_box(&b2);

    return 0;
}
Federico Ramponi
Thanks, I am always a Java guy and new to C. The idea of memory allocation is driving me crazy. May I know in main() how do I access each element of the particle array? like printf("%f", b1.p[1].x)
renz
Exactly (see the update).
Federico Ramponi
@ Federico, You might like to try the Boehem GC for c or c++ http://www.hpl.hp.com/personal/Hans_Boehm/gc/
philcolbourn
+1  A: 

You need to dynamically allocate struct Box.

struct Box *box1 = malloc(sizeof *box1 +
                          sizeof (Particle[number_of_particles]));

for (size_t i=0; i < number_of_particles; ++i)
    box1->p[i] = po[i];
/* or memcpy(box1->p, po, sizeof po); */

But if you are doing the above, you might as well declare struct Box to have a pointer in it. The flexible array member "trick" is useful if you want all the data in a struct to be contiguous. If you had a Point * pointer in the struct and allocated it dynamically, the memory layout for the struct and the points won't be contiguous.

The reason you are getting the error is that you can't assign to an array in C, and box->p is an array (from n1124 6.7.2.1p16):

However, when a . (or ->) operator has a left operand that is (a pointer to) a structure with a flexible array member and the right operand names that member, it behaves as if that member were replaced with the longest array (with the same element type) that would not make the structure larger than the object being accessed; the offset of the array shall remain that of the flexible array member, even if this would differ from that of the replacement array. If this array would have no elements, it behaves as if it had one element but the behavior is undefined if any attempt is made to access that element or to generate a pointer one past it.

You also need to have at least one other member in your struct other than the flexible array member.

Alok
+1  A: 

If you have a struct with a flexible array member, then you cannot directly create objects of that type, because the compiler does not know how long you want it to be. That's what you try to do with this line:

Box b1, b2;

Such structs always need to be allocated dynamically, which means that you only ever declare pointers to them. So you would change that declaration to:

Box *b1, *b2;

...and change your make_box() function to return such a pointer:

Box *make_box(int no_of_particles)
{
    Box *box = malloc(sizeof *box + no_of_particles * sizeof box->p[0]);

    if (box)
    {
        box->p[0].x = 1;
        box->p[1].x = 2;
        //so on and so forth...
    }

    return box;
}

void destroy_box(Box *box){
    free(box);
}

int main()
{
    Box *b1 = make_box(5);  //create a box containing 5 particles
    Box *b2 = make_box(10); //create a box containing 10 particles

    // Test that b1 and b2 are not NULL, then do the job...

    destroy_box(b1);
    destroy_box(b2);

    return 0;
}

PS: You also need to add at least one other member to the Box struct (no_of_particles would seem to be a good choice), because a flexible array member can't be the only member of a struct.

caf
Your last observation is a good point, but the first instruction of your make_box() doesn't make any sense to me. Better: Box *box = malloc(sizeof Box); if(box) { box->nparticles = no_of_particles; box->p = malloc(no_of_particles*sizeof(Particle)); if(box->p) { etc... Then void destroy_box(Box *box){ if(box){ if(box->p) free(box->p); free(box);}}
Federico Ramponi
Federico: The method I am using (and which the OP asked about) makes use of a *flexible array member*, which allows the entire thing to be allocated in a single allocation. Note that the type of the member `p` in `struct Box` is an incomplete array type, not a pointer. The GCC documentation on this is here: http://www.delorie.com/gnu/docs/gcc/gcc_42.html , although note that it *is* standard C.
caf
Wow! Sorry for my lame comment, it works. My issue was actually with the reference to "*box", and to "box->p" within the declaration of box itself. For that matter, even a declaration like "int a = sizeof(a);" would have been very suspect (but now I see it compiles correctly and does what is expected). Well, evidently my knowledge of C is more rusty and old than I thought. +1
Federico Ramponi
A: 

IF you are going to have a "flexible array", it MUST be the last element in the structure. This is simply because there is no way for the compiler to predict the offset of the next element. Additionally, when the structure is allocated, you will need to allocate EXTRA memory for your array elements.

Historical note: One thing that I used to see more of back in the day were practices like this ...

struct a {
  int x, y, z;
  char name[1];
};

struct a *  ptr;

ptr = (struct a*) malloc (sizeof (a) + EXTRA SPACE FOR name);

or

ptr = (struct a*) buffer;   /* <buffer> would have been a passed character array */

The above would allow one to access beyond the bounds of [name]. Perfectly legal and valid C, but potentially dangerous as you will be intentionally going beyond the originally declared bounds of [name].

Be careful out there.

Sparky