Back to General and Gameplay Programming

new is returning NULL

General and Gameplay Programming Programming

Started by Grain July 20, 2005 04:55 AM

19 comments, last by Sneftel 18 years, 9 months ago

Grain

500

Author

July 20, 2005 04:55 AM

This is an infuriating problem. The new operator is returning a null pointer. MSDN says it only doest that when there is insufficient memory, but I don’t see how that’s even remotely possible. For one thing its very early in my program and second when I check in task manager its only a bout 1MB used. What could possibly be causing this? I'll give you some code. class definition, this is the class I'm trying to make a new one of.


class OctTree
{
    OctTree *Parent;
    OctTree *Child[8]; 
    AABB B;
    unsigned int NumTris;
    unsigned int *index;
    vector3d* verts;
public:
    OctTree()
    {
        NumTris = 0;
        Parent = NULL;
        for(int i = 0; i < 8; i++)
        {
            Child = NULL;
        }
    }
    ~OctTree()
    {
        
        for(int i = 0; i < 8; i++)
        {
            if(Child) delete Child;
            Child = NULL;
        }
        if(Parent != NULL)
            delete []index;
        index = NULL;
    }
     void add_gemotry(vector3d* V, unsigned int num_verts, unsigned int *I,unsigned int num_tris);
// other methods...

};

This is the method where new is called and returns NULL


void OctTree::add_gemotry(vector3d* V, unsigned int num_verts, unsigned int *I, unsigned int num_tris)
{   
    verts = V;
    index = I;
    NumTris = num_tris;

    num_tris = 0;
    unsigned int *new_I = NULL;

    if(Parent == NULL)
    {
        B.add_points(V,num_verts);
    }

    if(NumTris > 1)
    {
        for(int i = 0; i < 8; i++)
        {
            AABB Box = B.eighth(i);

            for(unsigned int j = 0; j < NumTris; j++)
            {
                if( Box.contains(V[I[j*3]]) || Box.contains(V[I[j*3+1]]) || Box.contains(V[I[j*3+2]]))
                {            
                    num_tris++;
                }            
            }

            for(unsigned int j = 0, k = 0; j < NumTris; j++)
            {
                if( Box.contains(V[I[j*3]]) || Box.contains(V[I[j*3+1]]) || Box.contains(V[I[j*3+2]]))
                {
                    if(new_I == NULL) new_I = new unsigned int[num_tris];
                    new_I[k*3] = I[j*3];    new_I[k*3+1] = I[j*3+1];    new_I[k*3+2] = I[j*3+2];
                    k++;
                }
            }

            if(num_tris > 0)
            {
                if(Child == NULL)
                {Child = new OctTree;} // This is the culprit, it fails on the very first time through, so I know I'm not accidentally recursively creating a huge tree.
                Child->B = Box;
                Child->Parent = this;
                Child->add_gemotry(V, num_verts, new_I, num_tris);
            }
            num_tris = 0;
        }
    }
}

Oddly it works here.


class static_object :public object
{
    OctTree *Tree;
public:
    static_object()
    {
        Tree = new OctTree; // this works however
    }
    int LoadMS3DFile(char FileName[81])
    {
        if(Mesh::LoadMS3DFile(FileName))
        {
        Tree->add_gemotry(verts,num_verts,tris,num_tris);
        }
        else return 0;
        return 1;
    }
    vector3d colision_point(object &O);
    vector3d colision_normal(object &O);
    vector3d colision_normal(vector3d &V);
};

Basiror

241

July 20, 2005 05:48 AM

Just found this in your code, I personally prefer to use memset to set the entire array to NULL

also if you create an instance of a class you could use calloc and the placement new operator
that sets each byte to zero by default

for(int i = 0; i < 8; i++)        {            Child = NULL;        }

as for your code did you log the stderr maybe you are really running out of memory or just have a look at the taskmanager how much mbs your process allocated

p.s.: there is a little spelling mistake add_gemotry shouldn t it be add_geometry?

http://www.8ung.at/basiror/theironcross.html

Structure

240

July 20, 2005 07:01 AM

Ok I cant see anything wrong with your code and I tested in my own solution and it works fine, the only thing I can suggest is to make sure that your actually getting to that part of the code with break points and the new is actually getting called, otherwise sorry cant help.

[Edited by - Structure on July 20, 2005 7:01:34 AM]

[happy coding]

Basiror

241

July 20, 2005 07:40 AM

one thing i want to mention is you check if your child == NULL although you initialize it to NULL in the constructor

but instead you forget to check
if ((child= new octree...) == NULL)
{
perror("new failed");//include __file__ and __line__ maybe?
exit(EXIT_FAILURE);
}

http://www.8ung.at/basiror/theironcross.html

Grain

500

Author

July 20, 2005 07:51 AM

Quote:Original post by Basiror
as for your code did you log the stderr maybe you are really running out of memory or just have a look at the taskmanager how much mbs your process allocated

Yes I checked every way I know how, the programs memory usage right before the new call is little more than 1Mb and I have 1Gb on my system.

Quote:Original post by Structure
Ok I cant see anything wrong with your code and I tested in my own solution and it works fine, the only thing I can suggest is to make sure that your actually getting to that part of the code with break points and the new is actually getting called, otherwise sorry cant help.

Yes its getting called, I put a break point right before it.

Quote:Original post by Basiror
one thing i want to mention is you check if your child == NULL although you initialize it to NULL in the constructor

Yeah that’s just my coding habit, Always check for NULL be for calling new and Always set to NULL after delete, helps cut down on access violations and memory leaks.

Quote:Original post by Basiror
but instead you forget to check
if ((child= new octree...) == NULL)
{
perror("new failed");//include __file__ and __line__ maybe?
exit(EXIT_FAILURE);
}

Good Idea, but I already know its failing.

Oh and thanks for checking my spelling, that’s what I get for programming at 4am.

Sparhawk42

138

July 20, 2005 08:07 AM

There is one thing I would really check:
void OctTree::add_gemotry(vector3d* V, unsigned int num_verts, unsigned int *I, unsigned int num_tris)

has num_tris a sensible value...if you get here a zero new ist just doing what you ask of it, and if num_tris is really large, than you maybe have found your problem

By the way it is not good programming style, to change a parameter of your function, like you do with num_tris.

moeron

326

July 20, 2005 08:45 AM

Quote:Original post by Basiror
also if you create an instance of a class you could use calloc and the placement new operator
that sets each byte to zero by default

You have to be careful using memset or anything like that with classes, you don't want to wipe out your virtual function pointer table..

moe.ron

smr

2,468

July 20, 2005 09:01 AM

You spelled "geometry" wrong :D

I wouldn't use placement new or calloc when a simple memset(Child, 0, sizeof(Child) * sizeof(Child[0])); will do just fine.

You could also do memset(Child, 0, sizeof(Child) * sizeof(*Child))

Grain

500

Author

July 20, 2005 09:18 AM

Quote:Original post by Sparhawk42
There is one thing I would really check:
void OctTree::add_gemotry(vector3d* V, unsigned int num_verts, unsigned int *I, unsigned int num_tris)

has num_tris a sensible value...if you get here a zero new ist just doing what you ask of it, and if num_tris is really large, than you maybe have found your problem

I am loading a simple cube so num_tris is 12 as it shoud be.

Quote:Original post by Sparhawk42
By the way it is not good programming style, to change a parameter of your function, like you do with num_tris.

Although this is completely beside the point, what’s wrong with it? It’s passed by value so I can’t effect any thing out side the function, also once I have copied its value in to the member variable it’s served its purpose and is just sitting around doing nothing, so why bother creating another unsigned int who’s purpose will be to store the number of triangles I have. I just fits so nicely.

BTW: If it helps any, I put a break point in the OctTree constructor and its not being called on this line: if(Child == NULL){Child = new OctTree;} how ever the other one where the allocation works the constructor is called.

[Edited by - Grain on July 20, 2005 10:18:06 AM]

snk_kid

1,312

July 20, 2005 10:38 AM

Quote:Original post by Basiror
I personally prefer to use memset to set the entire array

Lets hope you don't do that on an array of NON POD-types in you're code, luckly for Grain an array of Pointers is a POD-type so using memset is okay but that is not the preferred method in C++.

Quote:Original post by Basiror
also if you create an instance of a class you could use calloc and the placement new operator that sets each byte to zero by default

Are you sure understand what placement new really does and when best to use it? if you did you would probably have never suggested such an idea of using calloc and placement new together. The whole point of seperating allocation/deallocation & construction/destruction is for improving efficiency in certain contexts not reducing it.

Quote:Original post by smr
I wouldn't use placement new or calloc when a simple memset(Child, 0, sizeof(Child) * sizeof(Child[0])); will do just fine.

You could also do memset(Child, 0, sizeof(Child) * sizeof(*Child))

As moeron mentioned you shouldn't use memset or any C memory rountines directly on NON POD-types but an array of pointers is a POD-type so using memset is okay but the best method in C++ is to use std::fill/fill_n as they are typically implementated to know the when POD-types are used and dispatches to use memset at compile-time when that is the case.

@Grain after a very brief glance at your code it seems to be okay however i do have some comments on it and improving it:

checking for null before invoking delete is redundant as a pointer passed to delete is permitted to be null.

In the C++ standard that overload of operator new should throw std::bad_alloc when it fails not return 0 that probably means exceptions have been disabled on your compiler, the no throw overload of operator new takes an instance of std::nothrow_t called std::nothrow and returns 0 to indicate failure.

Prefer using std::vector over C style dynamic arrays for you're vertex buffer. std::vecotr is random accessible and guaranteed to store elements in memory contiguously. To use it effectively read up about std:::vector::reverse method.

Prefer using C++ standard library algorithms over hand-written loops, it should make you're code more concise and clear, for example it appears your conditionally copying elements in this case one option is to use std::remove_copy_if

I recommend you separate tree nodes from the tree itself, tree nodes generally are/should not be responsible for memory management of there siblings or children etc. That should be taken care by the tree type not the nodes of it. Not only that but it significantly simplifies code having tree & tree nodes separate.

Optional: Instead of hard-coding operator new/delete, parameterize the tree by an allocator type and provide a default allocator type (std::allocator) like the standard library containers so you can provide custom allocator types like pooled allocators etc.

[Edited by - snk_kid on July 20, 2005 12:38:00 PM]

new is returning NULL

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

new is returning NULL

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines