Problems passing an array of pointers to objects to a function

I have a rather odd problem. I am trying to pass an array of pointers to objects to a function, which initializes and fills the array. It then returns the number of pointers to objects in the array. The code looks something like this:

Code:

---

class Material(){..}
 
class MaterialLoader(){
...
LoadMaterial(const char *path, Material *materials[] ){...}
}

---

The wierd thing is, when the LoadMaterial function is called from another class like this:

Code:

---

Material **materials = NULL;
MaterialLoader ml;
int num_materials = ml.LoadMaterial(path, materials);
printf("adress of materials is %x\n", materials);

---

the printf points to 0! The array is getting filled within the function, debugging the application as well as printf statements inside LoadMaterial confirm this. It is as though a copy is being made of materials, and that this is being passed to the function, but that's not possible, is it? And yet it must be, since that seems to be what is happening.

Can someone shed some light on this? If you want to compile the code, it can be obtained from http://code.google.com/p/mll/

Thanks in advance.

This is a simple model of your error:

Code:

---

void changeX( int x ) { x = 5; }
...
int x = 0;
changeX( x );
printf( "%d\n", x );

---

Do you see now why it isn't going to work?

It wouldn't work in the case you propose because you are not passing a pointer, so a copy is being made of x. Your analogy is wrong however, I feel. A better approximation of the problem would be:

Code:

---

void changeX( int *x ) 
{
x = new int;
*x = 5;
}
...
int *x;
changeX( x );
printf( "%d\n", *x );

---

which should work, since a pointer is being passed.

Alternatively, if there is indeed a copy being made, how can I avoid this? I normally pass pointers like this:

Code:

---

void changeX( int &x )

---

This avoids this problem. however, I can't find out how to pass an array of pointers to objects in this fashion.

Quote:

---

Originally Posted by Valheru

which should work, since a pointer is being passed.

---

It's a classical "passing pointers in C" problem. You'd have to pass a pointer to the pointer if you wanted to modify it. Currently you are passing a copy of the pointer which points to zero and that copy is initialized to the newly allocated chunk of memory, leaving the original pointer intact (set to zero or whatever else was there) so Jacek's example correctly shows what happens. The type used is irrelevant here.

A C++ way of doing what you want would be to pass a reference to the pointer. A C way of the same would be to pass a pointer to the pointer.

side note: print pointers with %p please, not with %x; this will safe you trouble on 64 bit platforms, among other things.


You declare materials to be an array of pointers (to Material).
Note the difference to "a pointer to an array of Materials".

Fix: three possibilities
1) (the best) use C++ containers like std::list, QList etc
2) pass Material ***materials to your function and initialize with
*materials = new Material *[num_materials]
change all the usages to (*materials)[cur_material]->...
3) decide that what you wanted was actually to pass a pointer to an array of Material objects:
change the call to: Material ** materials
then initialize with: * materials = new Materials[num_materials];
change the usages to (*materials)[cur_material]. ...



Basic idea:
An array (in C unfortunately) is basically the same as a pointer (suggestion: why don't you use std::list, or QList or something like that? Makes codes more readable and less error prone.)
So: your *Material[] is the same thing as Material**, it says: an array (or pointer to) Material-Pointers. It does not say a pointer to an array!

You want the function to initialize your pointer, so you pass a pointer to the pointer...

Then:
you have to set that pointer (with *materials = ) somewhere in the function you call.


Finally: This code does not make a lot of sense:

Code:

---

cout << "Initial adress of materials inside LoadMaterials is " << &materials << "(should still be 0)" << endl;

---

materials is a Material**, passed on the stack.
Therefore you are printing the address of a stack variable, here. This should not (and will not) be zero. Moreover, it is not interesting at all.
What you really want to print here is

Code:

---

materials

---

, not &materials.


Your code illustrates nicely that using several layers of pointers, in combination with C-style arrays is a nice way to make your head spin and produce buggy code. Do use C++ containers to make your life easier.
(The only reason not to use them is if you need to interface to C code.)

HTH

Quote:

---

Originally Posted by Valheru

I can't find out how to pass an array of pointers to objects in this fashion.

---

You can always ask c++decl:

Quote:

---

$ c++decl
Type `help' or `?' for help
c++decl> declare reference to array of pointer to int
Warning: Unsupported in C++ -- 'Reference to array of unspecified dimension'
(maybe you mean "reference to object")
int *(&var)[]
c++decl> declare reference to pointer to pointer to int
int **&var

---

(of course you will have to use the latter, as the warning says).

Although some coding styles say that if the function modifies its argument you should use a pointer instead of a reference, so that you can clearly see that in the code. Compare this two:

Code:

---

changeX( a, b, c, x );
changeX( a, b, c, &x ); // <- here you see exactly which argument is going to be changed

---

Ok, first : thanks to you all for your help. It's a tremendous help understanding all this.

Secondly, about my choice of container: I would like to keep it this way as opposed to using the infinitely easier vector solution. The reason for this is that I would like to pass the arrays later on to VBO's for example, and then they can be passed directly without any conversion.

As I understand it, probably the correct way to pass the array of pointers to Material objects would be:

Code:

---

LoadMaterials( const char &path, Material **&materials)

---

edit: Thanks, I have got it now. The original syntax is now valid passing Material **&materials in the function. I had expected to use "." accessors instead of "->" acessors though, but no. :)

Now I have an older problem back, namely that calling delete[] on a variable triggers an error report about HEAP CORRUPTION saying:

Code:

---

CRT detected that the application wrote to memory after end of heap buffer

---

Somewhere, I think I must be writing past the end of an array, but I can't for the life of me see where. :o

Please show us the piece of code ;-)
(The error probably is either on of: 1) the variable was not initialized, 2) does not point to memory allocated on the heap, 3) it is on the heap, but not an array, 4) double free. Hard to guess, though.)

Heh, I didn't expect you to guess though :p The complete code is available in the link in the first post: http://code.google.com/p/mll/source/checkout

It's not a terrible lot though. The place where it is going wrong is where I delete[] name and material_path, although the corruption is probably happening before that.

you create a vector of num_materials elements, and then add with push_back. That way you end up with a vector of 2x num_materials elements, I think.
Probably you want to call reserve, instead.

side note:

Code:

---

if( materials != NULL )
                delete materials;

---

These checks are not useful. Don't hurt either, though.

Oh God, what a stupid mistake :p

Code:

---

else if( buffer.substr( 0, 1 ) == "o" ){
            name = new char[buffer.length() - 2];
            sscanf(buffer.c_str(), "o %s", name);

---

I was allocating like that for certain variables. But I was not allowing space for the terminating \0 like that. It should have been buffer.length() - 1, obviously.

Quote:

---

Originally Posted by caduel

you create a vector of num_materials elements, and then add with push_back. That way you end up with a vector of 2x num_materials elements, I think.
Probably you want to call reserve, instead.

side note:

Code:

---

if( materials != NULL )
                delete materials;

---

These checks are not useful. Don't hurt either, though.

---

I thought that calling delete on a variable that has been set to NULL causes a segfault? I pass a lot of variables into the function set to NULL by default, since not every variable has to be defined in an OBJ file.

You allocate materials as a pointer to std::vector, but free it as an array with delete[].
Change that to delete and you should be ok.

HTH

Quote:

---

Originally Posted by caduel

You allocate materials as a pointer to std::vector, but free it as an array with delete[].
Change that to delete and you should be ok.

HTH

---

You lost me. I don't allocate anything as a vector. Could you point to the file and line that that happens according to you?

i) delete NULL is absolutely fine (just as free(NULL))

ii) When I browsed your svn repository online, I found such a line.
But it is not there. Perhaps I somehow git into an older revision. Sorry.

Any issues left?

Well, the program is still crashing on some files. I don't know why though, it does it when I call delete <var>. It differs per file. Sometimes it happens when I am deleting the normal_idx array, etc. I hesitate to use vectors, since with many allocations of models it would, I feel, introduce unwelcome memory usage - especially since there is no need to dynamically grow the arrays.

I'm trying to track it down now, although it may take a while. I'm learning a lot from it though, so I don't really mind too much, as long as it doesn't take too long :p