Andreas Mohr <a.mohr(a)mailto.de> writes:
objects/gdiobj.c really has no business dealing with SetLastError(). SetLastError() is a mechanism to communicate errors to external programs, so it should definitely not be used for highly internal functions.
You didn't include the gdiobj.c patch, but I disagree with it anyway... There can be many good reasons for setting last error in internal functions; for instance setting it inside GDI_GetObjPtr avoids having to set it in every single caller. And in the very rare case where this is not what we want, the caller can always set it to another value. -- Alexandre Julliard julliard(a)winehq.com
On Mon, Jun 04, 2001 at 11:11:23AM -0700, Alexandre Julliard wrote:
Andreas Mohr <a.mohr(a)mailto.de> writes:
objects/gdiobj.c really has no business dealing with SetLastError(). SetLastError() is a mechanism to communicate errors to external programs, so it should definitely not be used for highly internal functions.
You didn't include the gdiobj.c patch, but I disagree with it Hmm, strange. I did a diff with the files fetched from a pre-arranged file list, and I'm damn sure that I had included that file there :-\
anyway... There can be many good reasons for setting last error in internal functions; for instance setting it inside GDI_GetObjPtr avoids having to set it in every single caller. And in the very rare case where this is not what we want, the caller can always set it to another value. Now that's where I disagree ! The caller can *not* always set it to another value. If it does and the program intends to read the LastError of a *previously* executed API that failed, then the LastError will be reset even though it shouldn't have been ! (I know, programs that don't read the LastError immediately after function return are braindead, but then we're writing for the Windows platform, so we have lots of braindead programs anyway, right ? ;-)
In short: the SetLastError behaviour needs to match Windows behaviour very, very closely. And IMHO GDI_GetObjPtr() is such a widely used function that this will set LastError 6 in every case of a NULL handle, so this will spoil a lot of innocent error settings... Of course one could argue that it should be the calling function that should be fixed, then (to not pass a NULL handle to GDI_GetObjPtr). This would be SelectObject() in this case, BTW. What to do ? -- Andreas Mohr Stauferstr. 6, D-71272 Renningen, Germany Microsoft Windows: Made for the Internet The Internet: Made for Unix
On Mon, Jun 04, 2001 at 08:31:06PM +0200, Andreas Mohr wrote:
On Mon, Jun 04, 2001 at 11:11:23AM -0700, Alexandre Julliard wrote:
Andreas Mohr <a.mohr(a)mailto.de> writes:
objects/gdiobj.c really has no business dealing with SetLastError(). SetLastError() is a mechanism to communicate errors to external programs, so it should definitely not be used for highly internal functions.
You didn't include the gdiobj.c patch, but I disagree with it Hmm, strange. I did a diff with the files fetched from a pre-arranged file list, and I'm damn sure that I had included that file there :-\
anyway... There can be many good reasons for setting last error in internal functions; for instance setting it inside GDI_GetObjPtr avoids having to set it in every single caller. And in the very rare case where this is not what we want, the caller can always set it to another value. Now that's where I disagree ! The caller can *not* always set it to another value. If it does and the program intends to read the LastError of a *previously* executed API that failed, then the LastError will be reset even though it shouldn't have been !
There are only _some_ cases that do this.
(I know, programs that don't read the LastError immediately after function return are braindead, but then we're writing for the Windows platform, so we have lots of braindead programs anyway, right ? ;-)
In short: the SetLastError behaviour needs to match Windows behaviour very, very closely. And IMHO GDI_GetObjPtr() is such a widely used function that this will set LastError 6 in every case of a NULL handle, so this will spoil a lot of innocent error settings... Of course one could argue that it should be the calling function that should be fixed, then (to not pass a NULL handle to GDI_GetObjPtr). This would be SelectObject() in this case, BTW.
What to do ?
We need to evaluate LastError behaviour by function anyway. So if a function misbehaves, or sets the last error erroneously, fix it. Otherwise just do not care. Ciao, Marcus
participants (3)
-
520053692817-0001＠t-online.de -
Alexandre Julliard -
Marcus Meissner