Thread: Book- bad code :( me ish unhappyor

This is a function right out of the source code on the cd that came with "Isometric Programming with DirectX 7". It crashes with an exception error at the GetDC(). I don't know exactly what could be wrong, especially with the fact that this is totally uncharted territory for me. I tried using memset() on the hdc, but that wasn't the problem. Any help would be appreciated.

Code:

LPDIRECTDRAWSURFACE7 LPDDS_LoadFromFile(LPDIRECTDRAW7 lpdd,LPCTSTR lpszFileName)
{
	//load the bitmap
	CGDICanvas gdic;
	gdic.Load(NULL,lpszFileName);

	//create offscreen surface with same width and height
	LPDIRECTDRAWSURFACE7 lpdds=LPDDS_CreateOffscreen(lpdd,gdic.GetWidth(),gdic.GetHeight());

	//retrieve hdc for surface
	HDC hdcSurf;
	
	lpdds->GetDC(&hdcSurf);

	//blit image onto surface
	BitBlt(hdcSurf,0,0,gdic.GetWidth(),gdic.GetHeight(),gdic,0,0,SRCCOPY);

	//release dc
	lpdds->ReleaseDC(hdcSurf);

	//return surface
	return(lpdds);
}

My guess:

HDC GetDC(
HWND hWnd // handle to window
);

hWnd
[in] Handle to the window whose DC is to be retrieved. If this value is NULL, GetDC retrieves the DC for the entire screen.

This would suggest that you should have done:
>>HDC hdcSurf = NULL;
rather than
>>HDC hdcSurf;

Hammer:

Wrong GetDC(). He's calling IDirectDrawSurface7::GetDC()

http://msdn.microsoft.com/archive/de...ddref_9sxj.asp

CodeMonkey: Does it actually crash at that line, or does it crash at the next? Try going only to that line, then checking the return value of GetDC(). It may be that it is unable to get the device context you request, so the next line is attempting to use a DC which is invalid.

Also, are you sure the function LPDDS_CreateOffscreen is succeeding?? If you're trying to dereference a pointer to a surface that was never created, you'd crash for sure.

The first thing to start with is to check EVERY return code for every DirectX function. They should all return HRESULTs, log each to the screen or a file or whatever. It's possible something failed long ago, and that it takes until that point for the failure to actually crash the program. The bad part of book code is that it often doesn't check for all errors; the real world is not as forgiving, so check for every single error. Make SURE every DirectX function succeeds, don't assume it will.

>>Wrong GetDC().
Doh! My excuse: I guessed

Thanks Cat, Hammer. I'll do that. But just so we're clear, this is the error the debugger gives (attached).

That information leads me to suspect with much greater certainty that the problem is in LPDDS_CreateOffscreen or earlier.

As I suspected, the crash was an access violation; you were accessing memory you weren't allowed to. Most likely culprit? You dereferenced a pointer (in this case, lpdds) that didn't actually point to anything. And if lpdds doesn't point to anything, then the function failed.

My advice is the same, check all return codes. Figure out where the error actually is. It's not inconceivable to crash many hundreds of lines of code away from the actual error.

The problem could be within LPDDS_CreateOffscreen, or it could be a problem with gdic or lpdd. Maybe something as simple as lpszFileName is not a valid filename. The bottom line, you don't know that CGDICanvas::Load() succeeded, so you don't know that you can call functions on gdic and get meaningful results. You don't know that the DirectDraw even initialized OK (unless other functions actually check return codes), so you don't know lpdd is even a valid DirectDraw object. You're passing things which are possibly garbage into LPDDS_CreateOffscreen, so when you get garbage out, it's not easy to know where the problem lies.

If you had checked the error codes and knew for certain that lpdd was sucessfully created and gdic.Load() succeeded, then you'd know that the error was in LPDDS_CreateOffscreen(), but until you start checking all your return codes, you'll never know. All you really know is that LPDDS_CreateOffscreen is failing. It may be failing because the code within is wrong, or it may be failing because the parameters passed to it are bad.

So, not to sound like a broken record (if people these days even know what a record is or what a broken record sounds like) but I recommend you start by redirecting the clog stream to a file and outputting lots and lots and lots of debug information for yourself. E.g.:

HRESULT res;
clog << "Calling function xxx" << endl;
res = xxx();
clog << "Function xxx returns " << res << endl;

If you had done that for every single function call, you could find your problem in a matter of seconds.

Cat- I am a fool. Thanks for posting that last message, though, because it made me think of the image that was being loaded. I went to the function call and saw my error. I copied the source code to my hard drive, but not the single bitmap that needs to go with it.