Re: [PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:
> -
>    ok(!res && GetLastError() == ERROR_IO_PENDING, "%d: WaitCommEvent error %d\n", i, GetLastError());
> -
>    ok(res || (!res && GetLastError() == ERROR_IO_PENDING), "%d: WaitCommEvent error %d\n", i, GetLastError());

This change looks spurious and unrelated. Also todos must be removed
in the patch that fixes appropriate behaviour.

[PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

[Wolfgang Walter]

On Thursday 05 September 2013 10:42:40 you wrote:
> Wolfgang Walter  wrote:
> > -
> >    ok(!res && GetLastError() == ERROR_IO_PENDING, "%d: WaitCommEvent
> >
> > error %d\n", i, GetLastError()); +        ok(res || (!res &&
> > GetLastError() == ERROR_IO_PENDING), "%d: WaitCommEvent error %d\n", i,
> > GetLastError());
>
> This change looks spurious and unrelated. Also todos must be removed
> in the patch that fixes appropriate behaviour.

It is allowed that WaitCommEvent returns immediately with TRUE if there is an 
event pending, as far as I can see.

Regards,

[PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:
> On Thursday 05 September 2013 10:42:40 you wrote:
> > Wolfgang Walter  wrote:
> > > -
> > >    ok(!res && GetLastError() == ERROR_IO_PENDING, "%d: WaitCommEvent
> > >
> > > error %d\n", i, GetLastError()); +        ok(res || (!res &&
> > > GetLastError() == ERROR_IO_PENDING), "%d: WaitCommEvent error %d\n", i,
> > > GetLastError());
> >
> > This change looks spurious and unrelated. Also todos must be removed
> > in the patch that fixes appropriate behaviour.
>
> It is allowed that WaitCommEvent returns immediately with TRUE if there is an 
> event pending, as far as I can see.

WaitCommEvent always returns FALSE when device is opened in overlapped mode.

[PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 17:52:42 schrieb Dmitry Timoshkov:
> Wolfgang Walter  wrote:
> > On Thursday 05 September 2013 10:42:40 you wrote:
> > > Wolfgang Walter  wrote:
> > > > -
> > > >    ok(!res && GetLastError() == ERROR_IO_PENDING, "%d:
> > > >
> > > > WaitCommEvent
> > > > error %d\n", i, GetLastError()); +        ok(res || (!res &&
> > > > GetLastError() == ERROR_IO_PENDING), "%d: WaitCommEvent error %d\n",
> > > > i,
> > > > GetLastError());
> > >
> > > This change looks spurious and unrelated. Also todos must be removed
> > > in the patch that fixes appropriate behaviour.
> >
> > It is allowed that WaitCommEvent returns immediately with TRUE if there is
> > an event pending, as far as I can see.
>
> WaitCommEvent always returns FALSE when device is opened in overlapped mode.

Not according to MSN documentation of WaitCommEvent():

"If the overlapped operation cannot be completed immediately, the function 
returns FALSE and the GetLastError function returns ERROR_IO_PENDING, 
indicating that the operation is executing in the background."

Regards.

[PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:
> > WaitCommEvent always returns FALSE when device is opened in overlapped mode.
>
> Not according to MSN documentation of WaitCommEvent():
>
> "If the overlapped operation cannot be completed immediately, the function 
> returns FALSE and the GetLastError function returns ERROR_IO_PENDING, 
> indicating that the operation is executing in the background."

We have the tests for that, MSDN descriptions are proved to be often incorrect
or incomplete.

[PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 18:31:45 schrieb Dmitry Timoshkov:
> Wolfgang Walter  wrote:
> > > WaitCommEvent always returns FALSE when device is opened in overlapped
> > > mode.>
> >
> > Not according to MSN documentation of WaitCommEvent():
> >
> > "If the overlapped operation cannot be completed immediately, the function
> > returns FALSE and the GetLastError function returns ERROR_IO_PENDING,
> > indicating that the operation is executing in the background."
>
> We have the tests for that, MSDN descriptions are proved to be often
> incorrect or incomplete.

Then you think that wine behaviour should be changed? If yes I'll send a 
patch. But I wonder as wine does quit some effort to do otherwise.

Regards,

[PATCH 5/5] kernel32/tests: remove several todos in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:
> > > > WaitCommEvent always returns FALSE when device is opened in overlapped
> > > > mode.>
> > >
> > > Not according to MSN documentation of WaitCommEvent():
> > >
> > > "If the overlapped operation cannot be completed immediately, the function
> > > returns FALSE and the GetLastError function returns ERROR_IO_PENDING,
> > > indicating that the operation is executing in the background."
> >
> > We have the tests for that, MSDN descriptions are proved to be often
> > incorrect or incomplete.
>
> Then you think that wine behaviour should be changed?

If the test result is marked as todo_wine then yes.
> If yes I'll send a 
> patch. But I wonder as wine does quit some effort to do otherwise.

I'm already working on fixing bugs discovered by kernel32 comm and related
ntdll file tests.