C: Pointer to an array of structs inside a struct within a function

Question

I am pretty new to C. I created a struct and an array of structs to store data and passed each of them to my functions:

int dosomething(struct s1 *struct1, struct s2 *struct2);

struct S1 {
    int a;
    int b;
    int c;
};

struct S2 {
    double x;
    double y;
    double z;
};


int main()
{
    int n = 200;
    struct S1 s1;
    struct S2 *s2 = malloc(n * sizeof(struct S2));

    dosomething(&s1, &s2[0])

    return 0;
}

int dosomething(struct S1 *s1, struct S2 *s2)
{
    s1->a = 1;
    s1->b = 2;
    s1->c = 3;

    s2[0].x = 1.1;
    s2[1].x = 1.123233;
    ...

    return 1;
}

This got pretty annoying over time, so I decided to put them together. Now I am doing this

struct S1 {
    int a;
    int b;
    int c;
    struct S2 *s2;
};

struct S2 {
    double x;
    double y;
    double z;
};


int main()
{
    int n = 200;
    struct S1 s1;
    struct S2 *s2 = malloc(n * sizeof(struct S2));
    s1.s2 = s2;

    dosomething(&s1)

    return 0;
}


int dosomething(struct S1 *s1)
{
    s1->a = 1;
    s1->b = 2;
    s1->c = 3;

    s1->s2[0].x = 1.1;
    s1->s2[1].x = 1.123233;
    ...

    // OR

    struct S2 *s2 = s1->s2;
    s2[0].x = 1.1;
    s2[1].x = 1.123233;
    ...
}

Now, the thing is: I read a lot about pointers to structs, to arrays, to arrays of structs and so on, but I am still not sure, if what I am doing here is correct. C: pointer to array of pointers to structures (allocation/deallocation issues), c pointer to array of structs, How to declare pointer to array of structs in C, Pointer to array of struct in function. My project is growing to a size where I just don't want to make it worse by trying to make it better.

Do I need to make an array of pointers to the s2.s1[x]?

Do I need to tell the one pointer (struct S2 *s2 = s1->s2) how much elements s2 has?

Are there any pitfalls or could something go wrong?

Does your compiler show any warnings? About implicit declaration of `main()`? — Iharob Al Asimi, Aug 21 '17 at 16:29
What you have done seems fine from language point of view (it has no errors and has correct functionality), But design wise it can be made better — Ajay Brahmakshatriya, Aug 21 '17 at 16:30
@IharobAlAsimi It is just a minimal example I derived from my Situation. In my original code there is no such warning. Why do you think it could be? — dimple mind, Aug 21 '17 at 16:40
@dimplemind: it would be better to 1. post this on [Code Review](https://codereview.stackexchange.com/), and 2. use your **actual** code (which is a requirement for Code Review anyway). Using obfuscated names like S1 and S2 makes code reviewing and reasonable suggestions pretty much impossible. — vgru, Aug 21 '17 at 16:47
@Groo I understand your intention. But for me it was just a pretty basic difficulty without a big connection to the actual project. If I would post it on Code Review, I'd had to post 1300 rows of code (well, not all are relevant). But I didn't know Code Review exists, so I will have a look there. — dimple mind, Aug 21 '17 at 16:59
It doesn't have to be your entire program, just don't use hypothetical identifiers and be clear about *what* the program is supposed to do. You can remove the parts of `struct`s if it doesn't change the meaning. `do_stuff(&s1);` is bad, `calculate_taxes(&person);` is good. — vgru, Aug 21 '17 at 17:03
@BLUEPIXY You mean it shoulde be: `struct S2 (*s2)[n] = s1->s2;` ? — dimple mind, Aug 21 '17 at 17:09
E.g `int c; struct S2 *s2;` --> `int c; size_t n; struct S2 *s2;` ... `struct S2 *s2 = malloc(n * sizeof(struct S2)); s1.s2 = s2; s1.n = n;` — BLUEPIXY, Aug 21 '17 at 17:12

score 0 · Accepted Answer · answered Aug 21 '17 at 16:30

There is nothing wrong with it, it is a matter of whether or not it fits your requirements and that it is a good design choice in the long term.

Also, you can probably do

s1.s2 = malloc(sizeof *s1.s2);
if (s1.s2 == NULL)
    error_handler(); // do not use s1.s2 after this
dosomething(&s1);

score 0 · Answer 2 · answered Aug 21 '17 at 16:41

Placing a struct, pointer or an array inside another struct is called composition. You should be placing the S2 array inside the S1 struct if it belongs there.

For example, a Point is a pair of (x, y) coordinates, so it's obvious it should be composed of two ints, i.e.:

void do_stuff(struct Point pt);

Is better than:

void do_stuff(int x, int y);

But never do this just because you want to spare some typing. Do it if S2 fits inside S1. If "there is no S1 without S2" in your program, then it makes sense.

So if these two structs don't belong together, but you find yourself passing the same sequence of parameters to multiple functions, then you can use a variant of OOP refactoring technique called "introduce parameter object". I.e. instead of doing this:

do_stuff(&s1, &s2, &s3);

You can create a new struct whose no other purpose is but to be composed of these parameters:

struct StuffParameters {
    struct S1 * s1;
    struct S2 * s2;
    struct S3 * s3;
};

And then you can pass it to multiple "similar" functions:

struct StuffParameters parameters = { &s1, &s2, &s3 };

do_stuff(&parameters);
do_some_other_stuff(&parameters);
yet_another_function(&parameters);

Well, at the beginning I didn't know how to put the structs together and I wanted to have the code run. But they belong together. The "introduce parameter object" looks quite interesting. I have to remember that. — dimple mind, Aug 21 '17 at 16:50
@dimplemind: the idea with "introduce parameter object" (although C is not object-oriented) is a bit cooler than it first seems, as it allows you to extract all functions relevant to this set of parameters in a separate "module", while still leaving these structs logically separate for the functions where this makes sense. In C++, all these functions working on this new "parameter object" would be members of its separate class. But again, it would be better if you posted an actual chunk of your code to clarify what these structs actually represent. — vgru, Aug 21 '17 at 16:56
Today it is too late. I will leave in a minute. Maybe tomorrow I will post at least the relevant parts. — dimple mind, Aug 21 '17 at 17:06

C: Pointer to an array of structs inside a struct within a function

2 Answers2