Re: Crash in BeginPaint running Sams Photofact index, a free VB app
"Dmitry" == Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
Dmitry> "Dan Kegel" <dank(a)kegel.com> wrote: >> Backtrace: =>0 0x409e5873 (BeginPaint+0x9f(hwnd=0x10029, lps=0x0) >> [painting.c:200] in user32.dll.so) (ebp=40892bc4) Dmitry> Attached patch should help a bit. Dmitry> -- Dmitry. Protect BeginPaint and EndPaint from lps being NULL. Dmitry> --- cvs/hq/wine/dlls/user/painting.c Thu Jan 09 03:41:30 2003 Dmitry> +++ painting.c Mon Apr 28 10:15:46 2003 @@ -159,6 +159,8 @@ HWND Dmitry> full_handle; WND *wndPtr; Dmitry> + if (!lps) return 0; + if (!(full_handle = WIN_IsCurrentThread( Dmitry> hwnd ))) { if (IsWindow(hwnd)) @@ -239,6 +241,8 @@ */ BOOL Dmitry> WINAPI EndPaint( HWND hwnd, const PAINTSTRUCT *lps ) { + if Dmitry> (!lps) return FALSE; + ReleaseDC( hwnd, lps->hdc ); ShowCaret( Dmitry> hwnd ); return TRUE; Is lps==NULL an argument that BeginPaint() and EndPaint() should cope with or is it a result for errors hidden deeper? -- Uwe Bonnes bon(a)elektron.ikp.physik.tu-darmstadt.de Institut fuer Kernphysik Schlossgartenstrasse 9 64289 Darmstadt --------- Tel. 06151 162516 -------- Fax. 06151 164321 ----------
Is lps==NULL an argument that BeginPaint() and EndPaint() should cope with or is it a result for errors hidden deeper?
MSDN doesn't indicate any behaviour for when this is null. It just says the functions should fill out the struct that it points to. So passing NULL here is an error, that we have to protect ourselves against (clearly MS does) -- Mike Hearn <m.hearn(a)signal.qinetiq.com> QinetiQ - Malvern Technology Center
"Uwe Bonnes" <bon(a)elektron.ikp.physik.tu-darmstadt.de> wrote:
Is lps==NULL an argument that BeginPaint() and EndPaint() should cope with or is it a result for errors hidden deeper?
I've tested it before I sent the patch. Win2k just returns 0 for both APIs without a crash. -- Dmitry.
"Dmitry" == Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
Dmitry> "Uwe Bonnes" <bon(a)elektron.ikp.physik.tu-darmstadt.de> wrote: >> Is lps==NULL an argument that BeginPaint() and EndPaint() should cope >> with or is it a result for errors hidden deeper? Dmitry> I've tested it before I sent the patch. Win2k just returns 0 for Dmitry> both APIs without a crash. Ah, that's fine! Is your test perhaps integrable in our testsuite? Bye -- Uwe Bonnes bon(a)elektron.ikp.physik.tu-darmstadt.de Institut fuer Kernphysik Schlossgartenstrasse 9 64289 Darmstadt --------- Tel. 06151 162516 -------- Fax. 06151 164321 ----------
"Uwe Bonnes" <bon(a)elektron.ikp.physik.tu-darmstadt.de> wrote:
Dmitry> I've tested it before I sent the patch. Win2k just returns 0 for Dmitry> both APIs without a crash.
Ah, that's fine!
Is your test perhaps integrable in our testsuite?
I don't think that every trivial change to the code requires a test suite. -- Dmitry.
On April 28, 2003 07:13 am, Dmitry Timoshkov wrote:
"Uwe Bonnes" <bon(a)elektron.ikp.physik.tu-darmstadt.de> wrote:
Dmitry> I've tested it before I sent the patch. Win2k just returns 0 for Dmitry> both APIs without a crash.
Ah, that's fine!
Is your test perhaps integrable in our testsuite?
I don't think that every trivial change to the code requires a test suite.
This is one of my pet peeves. Wine is imperfect. (What isn't!) To me it is frequently unclear whether a line of code is present because it is needed to emulate (possibly weird) Microsoft behaviour or because we want our code to be better than Microsoft's or because of brainfade on the part of the programmer or ... That is why I like to see facts stated twice. Knowing which functions are supposed to handle null pointers is, to me, not trivial. (For example expand the question; is the function handling a null pointer or is it handling all segvs as Windows strlen does?) Putting in a test would be even better than simply commenting that the behaviour is known for e.g. Win95; it might point out version differences and certainly will catch when we accidentally break the code. -- Bill Medland ACCPAC International, Inc. medbi01(a)accpac.com Corporate: www.accpac.com Hosted Services: www.accpaconline.com
Putting in a test would be even better than simply commenting that the behaviour is known for e.g. Win95; it might point out version differences and certainly will catch when we accidentally break the code.
If we add a test for every micro-detail of the APIs, the test cases would probably be larger than Wine itself. I think sanity checking input parameters in this way is pretty obvious, for cases where it's not clear a comment in the code is probably more useful than another test case.
On April 28, 2003 11:59 am, Mike Hearn wrote:
Putting in a test would be even better than simply commenting that the behaviour is known for e.g. Win95; it might point out version differences and certainly will catch when we accidentally break the code.
If we add a test for every micro-detail of the APIs, the test cases would probably be larger than Wine itself. I think sanity checking input parameters in this way is pretty obvious, for cases where it's not clear a comment in the code is probably more useful than another test case.
Sorry but I have to disagree. "The obvious" is frequently not obvious. As I asked before (but in slightly different words), is the Microsoft behaviour to return this special value when passed a null pointer, or is it to catch a segv and return the special value? And does the special value returned vary between systems? It seems to me that half of what we are doing in Wine is figuring out the facts that are not in MSDN but are important. A test is a good way of documenting what we discover to be important, in a way that allows us to check our conformance. And what's the problem with a large test suite anyway? -- Bill Medland ACCPAC International, Inc. medbi01(a)accpac.com Corporate: www.accpac.com Hosted Services: www.accpaconline.com
participants (5)
-
Bill Medland -
Dmitry Timoshkov -
Mike Hearn -
Mike Hearn -
Uwe Bonnes