Re: user[2/2]: Preserve LastError.

2006-08-07 Thread Alexandre Julliard
Vitaliy Margolen [EMAIL PROTECTED] writes:

 Anything wrong with this patch? Test case clearly shows that
 we should preserve the LastError.

In general, saving/restoring last error is discouraged, the function
should be fixed to not change it at all. Do you have an app that
depends on this?

-- 
Alexandre Julliard
[EMAIL PROTECTED]




Re: user[2/2]: Preserve LastError.

2006-08-07 Thread Vitaliy Margolen
Monday, August 7, 2006, 3:52:51 AM, Alexandre Julliard wrote:
 Vitaliy Margolen [EMAIL PROTECTED] writes:

 Anything wrong with this patch? Test case clearly shows that
 we should preserve the LastError.

 In general, saving/restoring last error is discouraged, the function
 should be fixed to not change it at all. Do you have an app that
Sure but we call TlsGetValue() which does set LastError. So unless we can avoid
that call (which I doubt) we have to do something extra to preserve the
LastError.

 depends on this?
No I don't have any games directly. But I had few tests testing several things
in screen related areas and wasn't getting correct LastError. Also, looking at
the fact that x11drv doesn't set LastError at all we should preserve it as much
as possible. I haven't seen that many places in user32 that take care of
preserving LastError either.


Vitaliy.








Re: user[2/2]: Preserve LastError.

2006-08-07 Thread Alexandre Julliard
Vitaliy Margolen [EMAIL PROTECTED] writes:

 Sure but we call TlsGetValue() which does set LastError. So unless we can 
 avoid
 that call (which I doubt) we have to do something extra to preserve the
 LastError.

If an app depends on it, yes.

 No I don't have any games directly. But I had few tests testing several things
 in screen related areas and wasn't getting correct LastError. Also, looking at
 the fact that x11drv doesn't set LastError at all we should preserve it as 
 much
 as possible. I haven't seen that many places in user32 that take care of
 preserving LastError either.

The thing that matters is setting the proper error codes on
errors. Setting or not setting last error on success very rarely
matters, and both on Windows and Wine it will usually simply be a
side-effect of the functions being called internally.  

If we wanted to replicate the exact Windows behavior we'd essentially
need to manipulate last error in every single code path, and that
would be a real nightmare; so we should only do it if there's a
demonstrated need.

-- 
Alexandre Julliard
[EMAIL PROTECTED]




Re: user[2/2]: Preserve LastError.

2006-08-05 Thread Vitaliy Margolen
Saturday, July 29, 2006, 7:28:10 PM, Vitaliy Margolen wrote:
 ChangeLog:
 user: Preserve LastError.

  dlls/user/tests/input.c   |4 
  dlls/winex11.drv/x11drv.h |3 +++
  2 files changed, 7 insertions(+), 0 deletions(-)

Anything wrong with this patch? Test case clearly shows that
we should preserve the LastError.

Vitaliy Margolen







Re: user[2/2]: Preserve LastError.

2006-07-30 Thread Detlef Riekenberg
Vitaliy Margolen wrote:
 ChangeLog:
 user: Preserve LastError.

You can move the ok() for GetLastError() direct after GetKeyNameTextA():
 +SetLastError(0xdeadbeef);
  len = GetKeyNameTextA(i  16, buff, sizeof(buff));
 +ok( GetLastError() == 0xdeadbeef,
 +%d Wrong LastError = %ld\n, i, GetLastError());
  ok(len | !buff[0], %d: Buffer is not zeroed\n, i);

Just my 2 ¢


-- 
By By ...
  ... Detlef





Re: user[2/2]: Preserve LastError.

2006-07-30 Thread Vitaliy Margolen
Sunday, July 30, 2006, 6:29:43 AM, Detlef Riekenberg wrote:
 Vitaliy Margolen wrote:
 ChangeLog:
 user: Preserve LastError.

 You can move the ok() for GetLastError() direct after GetKeyNameTextA():
 +SetLastError(0xdeadbeef);
  len = GetKeyNameTextA(i  16, buff, sizeof(buff));
 +ok( GetLastError() == 0xdeadbeef,
 +%d Wrong LastError = %ld\n, i, GetLastError());
  ok(len | !buff[0], %d: Buffer is not zeroed\n, i);

 Just my 2 ¢
Those 2 ¢ would be spent better looking over wine-devel. What you proposed is
not correct.

Vitaliy