Strange variable problem

I have a variable which keeps changing for no reason.
I watched the variable with my debugger and in this code here -

Code:

---

  tree->nNodes++;
tree->nodes = (TREENODESTRUCT*)realloc( tree->nodes, sizeof( TREENODESTRUCT ) * tree->nNodes );
 tree->nodes[tree->nNodes-1] = *(TREENODESTRUCT*)lParam;

---

at treenNodes++ it goes from 0 to 1, which is right.
When it hits the last line it changes to some seemingly random number, like what you get before initializing the variable. I commented out the last line and it stays at 0, of course I also get a segfault because there is no info in the struct which I would be filling with the data in the lParam cast.

Looks like you are corrupting the heap somewhere. Can you check where you allocate memory for the tree structure?

tree = (TREESTRUCT*)malloc( sizeof( TREESTRUCT ) );

I'm out of ideas. I can only suggest checking all your memory allocation and make sure you are not overrunning any buffers.

>> I commented out the last line and it stays at 0 <<

Shouldn't it still be 1?

1. You've got the classic realloc bug there
p = realloc( p, n )
destroys p before you've had a chance to check for errors. If it returns NULL, you've lost everything

t = realloc( p, n ); if ( t != NULL ) { p = t; } else { panic(); do_something(p); }
The do_something() could be say save the user's data to a file.

2. non-obvious malloc/free bugs normally mean you've made a mess of it somewhere else in the code, and now you're seeing the delayed after-shock as it were.
Consider using some malloc debug tools to help you locate the real culprit

:| My god, if 1 is the case then I have loads of code to go and fix... :(

>> Shouldn't it still be 1? <<
Yeah I meant to say 1, good catch ;) ..

Yay it's working, thanks guys!

Seems I've spoken a bit to soon (who didn't see that coming?) ... It seems that all the child members of the tree are being changed. I only have one realloc and 2 malloc things though ... Here is all the code --

Code:

---

int DrawTree( HWND hwnd, TREESTRUCT *tree ) {
    HDC dc = GetDC( hwnd );
    HBRUSH brush = GetSysColorBrush( COLOR_WINDOW );
    RECT rc;
    GetClientRect( hwnd, &rc );
    FillRect( dc, &rc, brush );
    DeleteObject( brush );
    for( int i = 0; i < tree->nNodes; i++ ) {
        DrawText( dc, tree->nodes[i].lpText, -1, &tree->nodes[i].rc, DT_SINGLELINE | DT_LEFT | DT_VCENTER );
    }
    ReleaseDC( hwnd, dc );
    return 0;
 }
 
 int SizeTree( HWND hwnd, TREESTRUCT *tree ) {
    RECT rc = { 0, 0, 0, 0 };
    for( int i = 0; i < tree->nNodes; i++ ) {
      tree->nodes[i].rc.top = rc.bottom + tree->iSpaceY;
      tree->nodes[i].rc.bottom = rc.bottom + tree->iCharHeight;
      tree->nodes[i].rc.right = ( tree->iCharWidth * ( lstrlen( tree->nodes[i].lpText ) + 1 ) );
 tree->nodes[i].rc.left = (tree->nodes[i].parent < 0 ? tree->iSpaceX :tree->nodes[tree->nodes[i].parent].rc.left + tree->iSpaceX);
      rc.bottom = tree->nodes[i].rc.bottom;
    }
    return 0;
 }
 
 LRESULT CALLBACK TreeProc( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam ) {
    TREESTRUCT *tree = (TREESTRUCT*)GetWindowLong( hwnd, 0 );
    switch( msg ) {
      case WM_CREATE: {
        tree = (TREESTRUCT*)malloc( sizeof( TREESTRUCT ) );
        SetWindowLong( hwnd, 0, (LONG)tree );
        tree->bSet = false;
        tree->nNodes = 0;
        TEXTMETRIC tm;
        HDC dc = GetDC( hwnd );
        SetBkMode( dc, TRANSPARENT );
        GetTextMetrics( dc, &tm );
        ReleaseDC( hwnd, dc );
        tree->iSpaceX = 2;
        tree->iSpaceY = 2;
        tree->iCharWidth = tm.tmAveCharWidth;
        tree->iCharHeight = tm.tmHeight;
        tree->nodes = (TREENODESTRUCT*)malloc( sizeof( TREENODESTRUCT ) );
        return 0;
      }
      case M32_CREATENODE: {
        tree->nNodes++;
        TREENODESTRUCT* tmpPtr = (TREENODESTRUCT*)realloc( tree->nodes, sizeof( TREENODESTRUCT ) * tree->nNodes );
        if( tmpPtr != NULL ) {
          tree->nodes = tmpPtr;
          free( tmpPtr );
        }else{
          //Panic
        }
        tree->nodes[tree->nNodes-1] = *(TREENODESTRUCT*)lParam;
        SizeTree( hwnd, tree );
        return 0;
      }
      case WM_PAINT: {
        PAINTSTRUCT ps;
        BeginPaint( hwnd, &ps );
        DrawTree( hwnd, tree );
        EndPaint( hwnd, &ps );
      }
      case WM_DESTROY: {
        free( tree );
        return 0;
      }
    }
    return DefWindowProc( hwnd, msg, wParam, lParam );
 }

---

As you can see I'm trying to build a treeview control that works without recursion. The paint function is only a temp one till I get this damn bug sorted.

> tree->iCharWidth = tm.tmAveCharWidth;
> tree->iCharHeight = tm.tmHeight;
> tree->nodes = (TREENODESTRUCT*)malloc( sizeof( TREENODESTRUCT ) );
You don't need to malloc a 'dummy' node to start your empty array, you can simply start with
tree->nodes = NULL;
and realloc will behave just like malloc the first time you call it.

> if( tmpPtr != NULL ) {
> tree->nodes = tmpPtr;
> free( tmpPtr );
NO NO NO NO NO NO NO !!!!
Did I call free() in my example???
you REALLY don't want to free the memory you just allocated.

> TREESTRUCT *tree = (TREESTRUCT*)GetWindowLong( hwnd, 0 );
I hope you're not using index 0 for some other magic cookie elsewhere in your code. If you have lots of these, assign identifiers for each type using an enum.
Did you allocate the memory when you created your window class (see remarks)?
How about checking for errors when retrieving this important bit of information
Read

What does SizeTree() do?
It does some calculations, then throws the result away???

> tree->nodes[tree->nNodes-1] = *(TREENODESTRUCT*)lParam;
> SizeTree( hwnd, tree );
Two points
1. is lParam also allocated by the function which calls TreeProc?
If it is allocating memory, then you need to be more careful when you free a whole tree.
2. Why recalculate the size of the whole tree each time. Surely the new size is the size of the old tree (which you should remember) plus the size of the most recently added node.

> case WM_DESTROY: {
> free( tree );
As noted above, if each tree->node[i] is also pointing to allocated memory, that needs to be freed as well.
Shouldn't you also call
SetWindowLong( hwnd, 0, 0 );
to destroy your secret hidden copy of the pointer?

I wasn't quite sure about the free( tmpPtr ) I thought maybe you had just left it out, but I can see how it is a mistake.

I register the class with extra bytes and only the pointer to the tree struct is stored there.

free( tree ) is all I have done because I asked once before if I had to free any pointers in a pointer before but didn't get a reply so I just assumed not.

SizeTree sets the rect for each item in the tree so I can just traverse through the array and draw each item thus eliminating recursion. I size the tree again when a new node is created because the node can have a parent and so it's place can be anywhere in the tree not just at the bottom.

I create a node by doing this --

Code:

---

TREENODESTRUCT newNode;
newNode.image = NULL;
newNode.lpText = "Level 1";
newNode.parent = -1;
newNode.state = 0;
newNode.type = 0;
newNode.nNodes = 0;
 
SendMessage( hwndTree, M32_CREATENODE, 0, (LPARAM)&newNode );

---

It has no parent so it's parent member is set to -1, which makes it a root node.