1

I currently have a struct Struct1 which has a pointer to Struct2 that gets allocated multiple times based on some conditions in the code. I tried to allocate it test->Struct2Pair[i] = malloc(sizeof(struct Struct2));this way but it seems to fail. Any idea what I am doing wrong?

Here is a simplified version of what I'm trying to do.

struct Struct2 {
    int x;
    int y;
};

struct Struct1 {
    struct Struct2 *Struct2Pair;
    int val;
};

int main()
{

    struct Struct1 *test = malloc(sizeof(struct Struct1));
    for ( int i = 0; i < 5; i++ )
    {
        test->Struct2Pair[i] = malloc(sizeof(struct Struct2));
    }
    return 0;
}

Thanks!

Jane Doe
  • 27
  • 5
  • 2
    `Struct2Pair` is uninitialized -- you cannot just start assigning values to it as if it's a valid array. At the very least, you would need `test->Struct2Pair = malloc(5 * sizeof(struct Struct2));` which allocates memory for 5 elements of type `struct Struct2`. But if you're going to just hard-code an array, why not simply define it as `struct Struct2 Struct2Pair[5];` in the first place? – paddy Aug 09 '21 at 03:51
  • @paddy like I said that 5 is just an example. That number is variable based on some inputs and some computation. I just want to know how to malloc test->Struct2Pair each time some condition is satisfied. It can be 1... it can be 100. I'm unsure how to do that. – Jane Doe Aug 09 '21 at 03:55
  • @JaneDoe: When you do `test->Struct2Pair[i]`, you are trying to get access memory location of `Struct2Pair` which doesn't point to anything meaningful yet. – Sreeraj Chundayil Aug 09 '21 at 03:58
  • If you need to know how many items the array holds, then you need to store that value somewhere (an extra variable in `Struct1` perhaps). Otherwise you have no way of knowing if the array is large enough (if you plan to resize it later with `realloc` or whatever). I stand by my comment about how to allocate the array using `malloc`. You're doing it wrong. The only modification here is to use a variable size instead of hard-coding the `5`, and store the value somewhere. – paddy Aug 09 '21 at 04:00
  • It is runtime and not known while allocation of Struct2Pair. It keeps getting allocated one by one if certain conditions are satisfied. Would your solution still work in that case? – Jane Doe Aug 09 '21 at 04:04
  • Yes. It's your responsibility to keep track of the count and allocations accordingly. If you don't know the number of items in the beginning, you could set the count to zero and the pointer to NULL. Then any time you need to add an element, increase the count and call `realloc`. It's not the most efficient solution, but it will work and be the easiest for a beginner to understand. – paddy Aug 09 '21 at 04:04
  • Ah, makes sense. Thanks @paddy – Jane Doe Aug 09 '21 at 04:21

2 Answers2

0

In order to make Struct2Pair to point 5 consecutive memory object of type Struct2 you must allocate a 5*sizeof(Struct2). so in latter part of code you can access it as an array of size 5.

You must do null check for the return value of malloc.

    struct Struct1 *test = malloc(sizeof(struct Struct1));

        //   |-----change is here and here--------------|
        //   v                                          v
        test->Struct2Pair = malloc(sizeof(struct Struct2) * 5);
    
    //access can be as follows for all the index from 0  to 4
    test->Struct2Pair[2].x = 10;
    test->Struct2Pair[2].y = 20;

For better visualization refer this and this.

TruthSeeker
  • 1,539
  • 11
  • 24
-1

The issue you are having is located within the for loop and the number of available Struct2s in the test->Struct2Pair array.

You hadn't allocated any space for test->Struct2Pair before placing Struct2s into it

test->Struct2Pair[i] = malloc(sizeof(struct Struct2));

Then loop through more indexes then are available for the test->Struct2Pair array

for ( int i = 0; i < 5; i++ )

You may consider adding another variable to store the size of the test->Struct2Pair array:

int size = 5;
struct Struct1 *test = malloc(sizeof(struct Struct1));
test->Struct2Pair = malloc(size * sizeof(struct Struct2));
for ( int i = 0; i < size; i++ )
{
    test->Struct2Pair[i].x = 0;
    test->Struct2Pair[i].y = 0;
}

Reference on malloc: https://en.cppreference.com/w/c/memory/malloc

avonbied
  • 17
  • 4
  • That is incorrect. `Struct2Pair` is uninitialized. Accessing it would be UB. – alex01011 Aug 09 '21 at 04:02
  • Answer, acquire, and adapt. I eyeballed the question then adjusted the answer. Just opened a shell and tested the adjusted code and it compiles and runs. – avonbied Aug 09 '21 at 04:11
  • No problem @JaneDoe. I noticed realloc was mentioned without any reference. In case you do choose that route here's a general example of it's usage: https://en.cppreference.com/w/c/memory/realloc – avonbied Aug 09 '21 at 04:36
  • And here's another reference for dynamic memory management: http://www.eskimo.com/~scs/cclass/notes/sx11.html – avonbied Aug 09 '21 at 04:39
  • I did end up going the realloc way. Did `test->Struct2Pair = calloc(0, sizeof(struct Struct2));` first and then `test->Struct2Pair = realloc(test->Struct2Pair, (i+1) * sizeof(struct Struct2));` This seemed to solve my issues! – Jane Doe Aug 09 '21 at 05:43