> > + This->current = This->current +1; > > + HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, This->matrix, > > (This->current +1) * sizeof(D3DXMATRIX) ); > > + if ( This->matrix == NULL ) return E_OUTOFMEMORY;
> Aside from being a bit suboptimal (doing a realloc on every push), > this probably doesn't do what you want. Consider what happens to the > size of the array when you do something like push/pop/push/pop/...etc. > > It would be better to keep track of the current stack size, and then > grow it by a factor (eg. 2 or 1.5) if a push would overflow the > current stack. You could also consider shrinking the array again if a > pop would make it use less than a certain percentage of the current > size (eg. a third). > > You should also assign the result of HeapReAlloc() to This->matrix > again. Although it's quite possible for HeapReAlloc to just grow the > current block without changing its location, there's no guarantee it > will. The NULL check is useless this way. Just a little addition, I think we should resize the stack differently at different situations, i.e.: - resize it to (This->current+3) if This->currect<5 (something around that is the minimum number of matrices for which anyone would even bother using a MatrixStack) - resize it to (This->current*2) if This->current<12 (when it seems like the application's gonna push many matrices on the stack) - resize it to (This->current+16) if This->current>=12 (when *2 could waste memory) However, I don't know if these numbers are optimal, it's just a suggestion. For the Pop function, I agree that you should free some memory when it's unused, but my suggestion from above fits here, too. However, one third is okay, too, as it isn't that relevant here.
