Thread: Passing dynamically allocated structure array to function

Originally Posted by Matticus

It's likely you already know that the output of the following simple program will be 5.

Code:

#include <stdio.h>

void update(int x)
{
    x = 10;
}

int main(void)
{
    int x = 5;

    update(x);

    printf("%d\n",x);

    return 0;
}

We're passing a value to the function, so it's only a local copy that is changed - the value in the calling function remains the same.

It's the same idea with your struct pointer in "main()". You're passing that pointer to the function, which means it's only a local copy of that pointer that is changed - the pointer value in "main()" is not affected (and, incidentally, is not pointing anywhere meaningful when you use it on line 35).

To update a pointer in this situation, you need to pass the address of that pointer (a pointer to pointer).

---

That all being said, your overall approach to the problem seems generally flawed - as a result, I won't go into detail about the small problems I see.

You seem to be conflating pointers and arrays. While there are circumstances where they can seem to be used interchangeably, these are special cases - the two types are not universally compatible.

It looks like you're on the verge of discovering linked lists. I would suggest learning this data structure to achieve your goal.

Passing the address of the pointer to the function..
Don't you mean -

Code:

getInput(&Info,no);

where Info is the structure pointer?..
About arrays and pointers,
I've changed the function definition to

Code:

void getInput(struct test **s,int no);

and to assign the temp variable to the passed structure, I have used the following:

Code:

*(s+i) = t

I know about linked lists, but I would like to do this without using it and what exactly do you mean by flawed? bad logic? bad approach to problem or not using a data structure?

Originally Posted by anduril462

By flawed, I think he means bad logic, misunderstanding of how memory allocation works, failure to actually allocate all the memory you need (you have n students in the array, and each of those needs it's own malloc. You can assign the result of malloc directly to s[i] (hint, you must do this repeatedly, for each i up to number of students) and use scanf to read values directly into &s[i]->score or whatever the descriptive name you pick for the struct members is. You also should pick better names for your struct members and functions, and not use all caps (that is conventionally used for macros and constants). You should check the return value of malloc too, it can fail, returning NULL and causing a seg fault.

Replacing the existing code inside the for loop with the following, is this the correct way?

Code:

        
s[i] = malloc(sizeof(struct test));
scanf("%d%s%s%d",&s[i]->A,s[i]->B,s[i]->C,&s[i]->D);

Also, the size and the malloc of pointer that is passed into the function is done in the main function itself as

Code:

struct stud *Info = malloc(sizeof(struct test)*no);

But, again I am only able to access the first set of elements and not the following ones so again I think I am messing up the memory allocation ...

EDIT 1:
Instead of

Code:

&s[i]->A

Should I use this?

Code:

 &(*s)[i].roll

I can get the output for the nextset of values but, still I dont think it solved the allocation problem..

EDIT 2:
Similarly, changed s[i]->B to (*s)[i].b and it worked!
Thanks for the help anduril and Matt!

Originally Posted by anduril462

That doesn't seem correct, if indeed you have an array of pointers to struct tests -- you seem to be missing a level of indirection. You may get away with it, but it's likely to involve undefined behavior, or at the very least, the pointers you're dereferencing all have the right value, but you're using the wrong type. Similar to how

What is the correct way to access the address of the character array ?

Originally Posted by anduril462

If you're going off the end of the array somehow, or you're otherwise writing to the wrong place in memory, you get undefined behavior, so anything at all could happen. It's quite likely that some other operation overwrites the memory for the variable that stores the number of things in your array, possibly because of the "missing level of indirection" I mentioned.

Note, this is a really good case to use (or learn to use) a debugger.

Yes, overwriting the memory may be the problem, I will work on fixing that.

EDIT:
I tried the returning a double pointer method, it works well but for no reason the C[20] field gets changed to something random, others stay correctly most of the time but D(int) also gets messed up sometimes...
Am I trying to display it correctly?

Code:

printf("\nB :%s\nA:%d\nC:%s\nD:%d\n",Info[0]->B,Info[0]->A,Info[0]->C,Info[0]->D);

Info is again a double pointer to the structure.

Code:

struct test **Info;

EDIT: I missed your edit, as I was composing this reply, thus your returning a double pointer change is not accounted for.

Note, you're making this incredibly complicated on yourself. The suggestions I gave in post #7 would make this much cleaner. Any reason you must use an array of pointers? Any reason you can't use a VLA? Those two things would eliminate 90% of the pointer mess you're dealing with, plus they would the burden of making sure you free all this memory the right way.

So, if you want to make dynamic array of ints, you do something like

Code:

int *x = malloc(sizeof(int) * number_of_ints);

That is the type of the thing you are making an array of.
That makes a pointer to that type, which will point to the first element of the array.
That is the name of the variable.

So, instead of an array of int, we want an array of pointer to struct test, i.e. a struct test *. Thus, in main, you would declare the following:

Code:

struct test **tests;

Now, when we pass something to a function by reference, we have to pass it's address using the address-of operator. So that means that we will have one extra level of indirection in the function, and will have to dereference the variable before accessing the value it references. For example

Code:

void foo(int *x)
{
    *x = 42;
}
...
int x;
x = 17; // no need to do anything special, just use the variable
foo(&x);

That is the type of the thing we are changing.
That gives us a reference to (address of) the thing.
That is the name of the thing.
That is the dereference operator, allowing us to change the actual thing.
That is the address-of operator, passing the thing by reference (instead of value).

So, combining those two, we get the following (note the colors have the same meaning as above)

Code:

// receives a pointer to array of struct tests, and number of things to malloc
void getInput(struct test ***tests, int number_of_tests)
{
    *tests = malloc(sizeof(struct test *) * number_of_tests);
    (*tests);  // this will just give us the equivalent of tests in main, so we can use this like an array
    (*tests)[0];  // this is one element of the array, a pointer to struct test
    (*tests)[0] = malloc(sizeof(struct test));  // now we created a pointer to that struct test and store it in array element 0
}
...
// in main, or wherever
struct test **tests;
getInput(&tests, 42);  // pass tests by reference
tests[0]->score = 100;  // assign 100 to score of first test
free(tests[0]);  // you have to free all array elements
free(tests);  // then free the array itself

The only thing left is what you pass to scanf within getInput, so that you can set the value of a member of a struct test, pointed to by an element in your array

Code:

scanf("%d%s%s%d", &(*tests)[0]->score, ...);  // pass the member by reference

Remember, the numbers will need the & when passed to scanf. The strings will not, as they name of the struct member without any [ ] is already a pointer to the first element.

Confusing enough? I think you may now qualify as a three-star programmer. Again, I strongly suggest the simplifications I proposed in post #7.

Originally Posted by anduril462

Well, it will work fine if you do everything correctly. The issue is not really how you declare tests, or how you return it from the function. It's how you attempt to access the memory you have. Post the smallest compilable example that demonstrates the problem, and provide the input you are giving the program that causes the problem.

Note, you can also look into valgrind and electric fence, which are very powerful tools that help you find memory issues.

Here is the code (input method is same as what you have suggested in your previous post)

Code:

    struct stud{
        int r;
        char n[50];
        char b[50];
        int c;
    };


struct test** getInput(int number_of_tests)
{
    struct test **tests = malloc(sizeof(tests[0]) * number_of_tests);
    for (i = 0; i < number_of_tests; i++) {
        tests[i] = malloc(sizeof(tests[i]));
        scanf("%d%s%s%d", &tests[i]->r,tests[i]->n,tests[i]->b,&tests[i]->c);
        printf("r: %d n: %d b: %s c: %d",Info[1]->r,Info[1]->n,Info[1]->b,Info[1]->c); // just to compare
    }
    return tests;
}

int main(){
    int n = 2 // let us assume this is user input
    struct test **Info;
    Info = getInput(n);
    printf("r: %d n: %s b: %s c: %d\n",Info[0]->r,Info[0]->n,Info[0]->b,Info[0]->c);
    printf("r: %d n: %s b: %s c: %d\n",Info[1]->r,Info[1]->n,Info[1]->b,Info[1]->c);

}

If the input is

Code:

3
e
r
4

7
u
i
8

The output in the main function is:

Code:

3 e r 4
7 u d 8

Another set of example:

Input:

Code:

7
u
i
8

3
e
r
4

Output:

Code:

7 u i 8
3 e { 4

It does not happen 100% of the time, but it does so pretty often and every time it ONLY affects the value of character array 'b' of structure.

This looks wrong:

Code:

tests[i] = malloc(sizeof(tests[i]));

Generally, given a pointer p, allocating a single object for that pointer would be done by:

Code:

p = malloc(sizeof(*p));

So, given a pointer tests[i], allocating a single object for tests[i] would be done by:

Code:

tests[i] = malloc(sizeof(tests[i][0]));

or some equivalent statement.

I changed the malloc of test itself as:

Code:

struct test **t = malloc(sizeof(struct test*) * number_of_tests);

EDIT:
My mistake, that didn't work.. changing it to what laserlight suggested, it worked!
Also, it is same as the following?

Code:

tests[i] = malloc(sizeof(struct test));