[malloc]

In trying to compile for x86_64 I ran into this:

601 #ifndef HAS_SDL
602 *ppTexture = (LPDIRECT3DTEXTURE8)pTex;
603 #else
604 *ppTexture = (SDL_Surface*)pTex;
605 #endif
606
607 #ifdef HAS_XBOX_D3D
608 (*ppTexture)->Register(ResData);
609 #else
610 GetTextureFromData(pTex, ResData, ppTexture);
611 #endif
612 *(DWORD*)(pTex + 1) = (DWORD)(BYTE*)UnpackedBuf;

Line 604 is terribly unportable because ppTexture contains pointers which change the size and therefore change how it's mapped onto the pTex structure. Not only that but it seems to be completely useless. In the call on line 610 the current state of ppTexture is completely ignored.

Then on line 612 we do another thing that's unportable because pointers are 64 bits on the x86_64 architecture. In addition, this seems to be completely useless because the as far as I can tell the pTex memory is just leaked after the function returns a few lines below this! pTex is local, it's never freed, and it doens't appear to be referenced. What is going on?

I've even commented out 604 and 612 and it seems to have no consequence on an 32 bit build.

Similar code is used in LoadAnim on lines 714-727. This probably does something on the XBOX, but not in Linux (opinion!).

Thoughts? Otherwise I may just "fix" this in my x86_64 patch.

[malloc]

Two more thoughts:
1. In LoadAnim the memory for ppTex is actually freed at the end. This seems to confirm my assumptions that line 612 is useless assuming an analogous case.

2. By the time we get to line 612, ppTexture no longer refers to pTex (GetTextureFromData allocates some new memory for it), so it isn't the case that we're attempting to write into that memory either.

[jmarshall]

Correct. For any non-xbox build the code is completely unnecessary as well as being completely wrong On xbox we're simply assigning directly to the memory block we've uncompressed to and fixing up the tex header so that the xbox knows it's got a real texture (i.e. saves us asking for a texture and doing another malloc/memcpy). On any other platform, we will be doing another malloc and transfer of the memory content.

Please do clean it up and post a patch

Cheers,
Jonathan

[malloc]

On a related note what's the difference between HAS_XBOX_D3D and !(HAS_SDL)? It would seem to me that at this point in time all supported plateforms use one or the other in a disjoint fasion. I can't imagine any platform except the Xbox would define HAS_XBOX_D3D. Perhaps Windows defines it and it's named such for legacy reasons. I can imagine in the future having a platform which uses neither XBOX_D3D nor SDL, but I can't imagine a platform using SDL and XBOX_D3D (or some combination with an additional graphics library). So why on earth do we have things like "ifndef HAS_SDL" mixed with "ifdef HAS_XBOX_D3D"?

[jmarshall]

Not quite - the win32 build in trunk uses DirectX, so that's !HAS_XBOX_D3D and !HAS_SDL.

But yeah, probably could be organized somewhat better.

Cheers,
Jonathan