Patch for bug 34388

[Charles Davis]

Hi Juan,

I need some advice.

Attached is a patch to fix bug 34388, where an app that tries to verify its 
code signature fails because Wine, while parsing the certificates embedded 
within the signature, encounters an item in the CMS signer info (tag 0x31, i.e. 
ASN_UNIVERSAL | ASN_CONSTRUCTOR | 0x11), right before the 
HashEncryptionAlgorithm sequence item, that it doesn't yet know how to decode. 
This patch (which just skips the item in question) allows that app to 
successfully verify its code signature and run, but...

My testing on Windows (cf. job 2037 on newtestbot) shows some interesting 
behavior. Windows will indeed accept this item, but only if the AuthAttrs item 
is also present and immediately precedes it in the sequence. (The other test 
bails out with CRYPT_E_ASN_BADTAG.) I don't quite know what to make of this. 
The odd thing is that the certificate in question doesn't have this optional 
AuthAttrs item, and yet (in most cases, at least) most people who run this 
particular app on Windows do not have this problem. I can't seem to find 
anything about this from reading the relevant RFCs. Is there something I'm 
missing?

Thanks.

Chip


0001-crypt32-Skip-unknown-item-when-decoding-a-CMS-certif.patch
Description: Binary data

Re: [PATCH 1/3] riched20: Use codepage in ME_ToUnicode. (try 2)

[Nikolay Sivov]

On 9/4/2013 21:16, Jactry Zeng wrote:

Hi Nikolay,

2013/9/5 Nikolay Sivov >

>
> On 9/4/2013 07:17, Jactry Zeng wrote:
>>
>> -ME_EndToUnicode(unicode, wszText);
>> +ME_EndToUnicode(unicode ? 1200 : CP_ACP, wszText);
>
> It's still ugly to use magic numbers like that, in a similar 
situation I had some time
> ago I use ~0 to mark WCHAR data encoding that does need special 
handling.

> Even with a value like ~0 it needs to be documented in code comments.
>

Thanks for your review!
But I still can't understand it. :(  Can you show me a commit as an 
example.
What I mean is it's better to use something more neutral to mark an 
absence of

codepage.

In my case it was like that 
http://source.winehq.org/git/wine.git/commit/320d419be1fe713c50eac7d53e4dc52481fad8b7 



Thank you!

(sorry for forgeting cc wine-devel)
--
Regards,
Jactry Zeng

Re: [PATCH 2/3] ws2_32: Implement get socket option SO_PROTOCOL_INFO (try 3)

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
https://newtestbot.winehq.org/JobDetails.pl?Key=2035

Your paranoid android.


=== wxppro (32 bit sock) ===
sock.c:519: Test failed: oob_server (b8): unexpectedly at the OOB mark: 0
sock: unhandled exception c005 at 

Re: crypt32 patch review

[Juan Lang]

[+wine-devel]

Hi Jacek,
I've added the list so the discussion can take place in public.

On Tue, Sep 3, 2013 at 8:50 AM, Jacek Caban  wrote:

> I have a patch for crypt32. I'd appreciate your review before I submit
> it to Wine. It has high potential to be insecure... This should fix Wine

bug 28004 as well as some websites that don't send full cert chain in
> SSL/TLS handshake, but instead just their own certs. For that we need
> two changes. First, we need to look for issuers in global stores (as in
> those that are not passed to CertGetCertificateChain). When I did that,
>

I don't really see a security issue here. The global stores are implicitly
trusted, so neglecting to check them was probably an oversight on my part.
Your tests imply something is missing.

My only real question here is whether the test results are in any way
impacted by some unknown caching happening in Windows. Has this chain been
verified prior to running the test? I'm going to guess that the test bot
machines are "clean" prior to running the tests, but if they download their
test executables from winehq.org, that won't be the case.


> it came out that it's not enough for intermediate issuers. Those have to
> be downloaded if we have information about their location stored in
> validated cert. That's easy using cryptnet. Could you please review the
> attached patch?
>

+ for(i=0; i < info->cAccDescr; i++) {
+if(!strcmp(szOID_PKIX_CA_ISSUERS,
info->rgAccDescr[i].pszAccessMethod)
+&& info->rgAccDescr[i].AccessLocation.dwAltNameChoice ==
CERT_ALT_NAME_URL) {

You explicitly make use of the AIA extension to find the location of
intermediate certs. This is reasonable, and it gets your test case to pass,
but it won't cover end certs that don't make use of the AIA extension. As I
said above, I wonder whether caching is impacting the test results. More:

The thing that would be also nice to have as follow up is to cache
> downloaded URLs. They are already cached in URL cache, but I believe
> that they should be also cached in some place like world collection or
> something like that. I'm not sure what would be appropriate. Do you have
> a suggestion?
>

Yes. I suspect the most relevant bug is actually 27168. 28004 is probably a
dupe of it, except that I'm not certain that PartyPoker uses chromium. In
brief, Microsoft's crypt32 appears to cache intermediate certs in a
PCCERT_CONTEXT's hCertStore. What I should be doing in crypt32, but am not,
is to have certs ref count the stores they're in, such that the last cert
to be closed causes its store to be freed, if the store is closed prior to
the cert being freed. This, combined with certificate chains being cached
in Microsoft's implementation, would allow intermediate certs to be found
without explicitly checking the AIA.

You can see how the certificate chain engine ought to support caching on
MSDN:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa377184(v=vs.85).aspx

I never implemented chain caching, as I expected it'd be a performance
optimization and could be addressed at a later time, and never got back to
it.

In addition to bug 27168, you can see how Chromium is making use of
crypt32's certificate caching in
net::X509Certificate::CreateOSCertChainForCert, both its comments and
implementation:
http://src.chromium.org/viewvc/chrome/trunk/src/net/cert/x509_certificate.h
http://src.chromium.org/viewvc/chrome/trunk/src/net/cert/x509_certificate_win.cc

Back to your patch: I'm not entirely opposed to the explicit AIA approach,
but I think it's worth at least thinking about other methods. And, I'd
prefer to see the code block more explicitly separated from the surrounding
code, with either a comment or a clear function name to indicate what's
going on. As it is, one has to read quite a bit of CRYPT_FindIssuer to
realize that it's really looking for an AIA extension to find intermediate
issuers.

Last thing on the patch, some style nits. (You already remarked that it
needs splitting, and I agree.)

+if(issuer) {

 Elsewhere in crypt32 I indent prior to open parentheses, so I worry this
will make it a little harder to maintain a consistent style. Please observe
the existing style, here and elsewhere.

-issuer = CertFindCertificateInStore(store,
- subject->dwCertEncodingType, 0, CERT_FIND_SUBJECT_NAME,
- &subject->pCertInfo->Issuer, prevIssuer);
+issuer = CRYPT_FindIssuer(engine, subject, store,
CERT_FIND_SUBJECT_NAME,
+  &subject->pCertInfo->Issuer, flags,
prevIssuer);

It might not be easy to guess my indenting style for continuation lines,
but I use a single additional space to indicate continuation through
crypt32. I'd appreciate observing the same style.

- hAdditionalStore, &chain);
+hAdditionalStore, dwFlags,
&chain);

Likewise, the indentation change here doesn't seem necessary.

Thanks very much for working on this. Hope my comments ar

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Jonas Maebe]

On 05 Sep 2013, at 16:58, Dmitry Timoshkov wrote:

> Jonas Maebe  wrote:
> 
>> So either
>> * you add code in between that resets the state so that test2 can be run 
>> independently of whether test1 failed (as Wolfgang proposes), or
>> * you put the tests in completely different executables so they don't 
>> influence each other (hopefully), or
>> * you reorder the test so that first all the tests that currently work under 
>> wine are run (under the assumption that no regression will ever creep in), or
>> * you require that bugs in wine routines are fixed in the order that they 
>> are called in that test (which would be strange)
> 
> How about a patch set which does:
> 1/2. add a workaround to the tests to pass under Wine
> 2/2. fix Wine code and simultaneously remove the workaround added by 1/2.
> 
> But that looks even more strange if the fix contained in 2/2 could be
> sent alone.


I disagree it's a "workaround to make the tests pass under Wine". It's simply 
good practice to make supposedly unrelated tests actually independent from each 
other. The fact that this makes some extra tests succeed under Wine simply 
means that the test code was broken before (unless the only goal was to test 
whether everything works or not everything works, as opposed to testing whether 
individual functionality works). And when a regression occurs afterwards, it 
will also be easier to see what exactly broke by the regression rather than 
having to add all that code again to make the tests independent from each other 
once more.


Jonas

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 23:58:57 schrieb Dmitry Timoshkov:
> Jonas Maebe  wrote:
> > > The tests must pass under Wine without any additional "fixes" as they do
> > > currently under Windows. If you add some code to the tests which
> > > suddenly
> > > makes them pass under Wine - that's not a fix, Wine implementation
> > > should
> > > be fixed instead.
> > 
> > What I understood from Wolfgang's previous explanations is that there is
> > currently this situation:
> > 
> > todo_wine
> > 
> >  test1();
> > 
> > /* test1 fails because wine has a bug. As a side-effect, bytes will be
> > left in the transmission buffer under wine, but not under windows. */
> > test2();
> > /* test2 also fails under wine because of the failure of test1 that
> > influences the behaviour of test2, not because the functionality tested
> > by test2 is broken. This make it impossible to individually evaluate
> > test1() and test2(). */
> > 
> > So either
> > * you add code in between that resets the state so that test2 can be run
> > independently of whether test1 failed (as Wolfgang proposes), or * you
> > put the tests in completely different executables so they don't influence
> > each other (hopefully), or * you reorder the test so that first all the
> > tests that currently work under wine are run (under the assumption that
> > no regression will ever creep in), or * you require that bugs in wine
> > routines are fixed in the order that they are called in that test (which
> > would be strange)
> How about a patch set which does:
> 1/2. add a workaround to the tests to pass under Wine
> 2/2. fix Wine code and simultaneously remove the workaround added by 1/2.

I don't have a fix for

WaitCommEvent() does not return ERROR_INVALID_PARAMETER without ovl 
structure

And I had no plans to do so nor did I claim.

I removed the side effect that this test makes the next test fail which tests 
the EV_TXEMPTY handling even if that it would work perfectly.

So:

test A
test B

test A fails on wine and this influences test B in such a way that it will 
always fail, too.

I remove this dependency.

Now test B succeeds. This is not because I worked arround a wine-bug in what B 
should test but because wine behaves correctly in this test.

And test A still fails.

> 
> But that looks even more strange if the fix contained in 2/2 could be
> sent alone.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 23:32:00 schrieben Sie:
> Wolfgang Walter  wrote:
> > > > > > With my patches 1/2 and 2/2 I get:
> > > > > > 
> > > > > > fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped
> > > > > > 0x10ee8ec):
> > > > > > stub wine: configuration in '/home/walterw/.wine' has been
> > > > > > updated.
> > > > > > fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented
> > > > > > stub
> > > > > > comm.c:857: Test succeeded inside todo block: WaitCommEvent failed
> > > > > > with a
> > > > > > timeout comm.c:881: Test succeeded inside todo block:
> > > > > > WaitCommEvent
> > > > > > error
> > > > > > 997 comm.c:883: Test succeeded inside todo block: WaitCommEvent:
> > > > > > expected
> > > > > > EV_TXEMPTY, got 0x4 comm.c:889: Test succeeded inside todo block:
> > > > > > WaitCommEvent used 1141 ms for waiting err:comm:set_line_control
> > > > > > ByteSize
> > > > > > fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b
> > > > > > access=0
> > > > > > func=3 method=0) comm.c:2036: Tests skipped: interactive tests
> > > > > > (set
> > > > > > WINETEST_INTERACTIVE=1)
> > > > > 
> > > > > Successes inside of todo_wine blocks are treated as failres.
> > > > 
> > > > So you think I should remove the wine_todos already here?
> > > 
> > > No, the source of the failures is still there.
> > 
> > What do you mean with that? The tests indeed do succeed now and there is a
> > reason they do: when you call WaitCommEvent() while the tx buffer is not
> > empty
> > yet the wine code will detect that EV_TXEMPTY correctly:
> The tests must pass under Wine without any additional "fixes" as they do
> currently under Windows. If you add some code to the tests which suddenly
> makes them pass under Wine - that's not a fix, Wine implementation should
> be fixed instead.

You basically say that one may not fix a bug before fixing another one which 
is not related only because they are tested for in a special order and these 
tests influence each other though they are not really related.

Again:

In this case not wine behaves incorrectly but the test is simply wrong. It 
first tests if WaitCommEvent() returns  ERROR_INVALID_PARAMETER if called with 
NULL without without ovl structure. Then it tests the EV_TXEMPTY handling. The 
latter test will always fail even if it works perfectly as long as wine does 
not return ERROR_INVALID_PARAMETER but instead does the write in the first 
test. This is because the second tests does not wait long enough for both 
writes to complete.


Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Jonas Maebe  wrote:

> > The tests must pass under Wine without any additional "fixes" as they do
> > currently under Windows. If you add some code to the tests which suddenly
> > makes them pass under Wine - that's not a fix, Wine implementation should
> > be fixed instead.
> 
> What I understood from Wolfgang's previous explanations is that there is 
> currently this situation:
> 
> todo_wine
>  test1();
> /* test1 fails because wine has a bug. As a side-effect, bytes will be left 
> in the transmission buffer under wine, but not under windows. */
> test2();
> /* test2 also fails under wine because of the failure of test1 that 
> influences the behaviour of test2, not because the functionality tested by 
> test2 is broken. This make it impossible to individually evaluate test1() and 
> test2(). */
> 
> So either
> * you add code in between that resets the state so that test2 can be run 
> independently of whether test1 failed (as Wolfgang proposes), or
> * you put the tests in completely different executables so they don't 
> influence each other (hopefully), or
> * you reorder the test so that first all the tests that currently work under 
> wine are run (under the assumption that no regression will ever creep in), or
> * you require that bugs in wine routines are fixed in the order that they are 
> called in that test (which would be strange)

How about a patch set which does:
1/2. add a workaround to the tests to pass under Wine
2/2. fix Wine code and simultaneously remove the workaround added by 1/2.

But that looks even more strange if the fix contained in 2/2 could be
sent alone.

-- 
Dmitry.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:
 
> > > > No, the source of the failures is still there.
> > > 
> > > What do you mean with that? The tests indeed do succeed now and there is a
> > > reason they do: when you call WaitCommEvent() while the tx buffer is not
> > > empty
> > > yet the wine code will detect that EV_TXEMPTY correctly:
> > The tests must pass under Wine without any additional "fixes" as they do
> > currently under Windows. If you add some code to the tests which suddenly
> > makes them pass under Wine - that's not a fix, Wine implementation should
> > be fixed instead.
> 
> You basically say that one may not fix a bug before fixing another one which 
> is not related only because they are tested for in a special order and these 
> tests influence each other though they are not really related.

Adding a workaround to the tests to compensate a Wine bug and as a side effect
remove some todo_wines is not a fix. Yes, some tests depend on each other, but
that's on purpose, there is no need to break this dependency just because you
can make some later tests suddenly "pass".

-- 
Dmitry.

D3D command stream patches for testing

[Pierre Franco]

I've tested some games (wine 1.7.1 64-bit for Guild Wars 2 and SCII, 32-bit
for Skyrim, no wined3d/d3d patch except the patchset for CSMT).

I have a GTX660 (slightly overclocked, +10 Mhz or so) with the 325.15
Nvidia blob and an Intel i5 2500K (overclocked to 4.2 Ghz).

#Guild Wars 2(in the Lion's Arch with ~75-100 players, always from the same
spot):

Quality Options:
 -Fullscreen Mode
 -1600x1200
 -Animation: High
 -Anti-aliasing: FXAA
 -Environment: High
 -LOD: Excellent
 -Textures: High
 -Reflections: Land and Sky(causes high frame rate drops when moving
the camera)
 -Render Sampling: Supersampling
 -Shadows: High
 -Shaders: High
 -Post-processing: High(absolutely doesn't influence frame rate)
 -No High Res Character Textures
 -Character model limit/Character model quality: Medium
 -VSyng: Off
 -Optimal Texture Filtering: Off(no real effect except reducing the FPS)

Without -dx9single(it should use more than one core and DX10 then):
 -With __GL_THREADED_OPTIMIZATIONS: 30-45 FPS in the login screen, 8-10
FPS when not moving the camera, 3-5 FPS when moving it.
 -Without __GL_THREADED_OPTIMIZATIONS: 60 FPS in the login screen,
18-20 FPS when not moving the camera, 5-10 FPS when moving it.

With -dx9single(the game is far more quicker to load with it):
 -With __GL_THREADED_OPTIMIZATIONS: >60 FPS in the login screen, 23-24
FPS when not moving the camera, 5-20 FPS when moving it.
 -Without __GL_THREADED_OPTIMIZATIONS: >60 FPS in the login screen,
23-24 FPS when not moving the camera, 20 FPS when moving it.

Conclusion: -dx9single is GOOD and __GL_THREADED_OPTIMIZATIONS is BAD.

#Starcraft II

Quality Options: Ultra(default mode, also for the textures).
The game claims 50-70 FPS, but it seems to lag when moving the camera and
the output is only trash (I need to change a graphical setting to be able
to see anything).
With __GL_THREADED_OPTIMIZATIONS, it's even more laggy and the FPS drop
below 40.

#Skyrim

Quality Options: Very High.
I can't see the FPS of the game, but it was running smoothly and is still
running smoothly with the patchset.
I didn't tried with __GL_THREADED_OPTIMIZATIONS(well, I tried long ago, and
that was horribly slow).

I also want to say that wine is spamming "Waiting for the cs" in the
console (I assume it's nothing really important as the project seems to be
WIP, why asking for tester otherwise?) and wgl errors (err:wgl:wglFlush
wglFlush).

Cordially.

P.S. : Sorry for my English, I am still learning it.
-- 
Pierre Franco
nob.dir.i...@gmail.com

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

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
https://newtestbot.winehq.org/JobDetails.pl?Key=2023

Your paranoid android.


=== w7pro64 (32 bit comm) ===
comm.c:856: Test failed: WaitCommEvent failed with a timeout
comm.c:868: recovering after WAIT_TIMEOUT...
comm.c:879: Test failed: WaitCommEvent error 0
comm.c:880: Test failed: WaitCommEvent: expected EV_TXEMPTY, got 0
comm.c:884: WaitCommEvent for EV_TXEMPTY took 2137 ms (timeout 2133)
comm.c:885: Test failed: WaitCommEvent used 2137 ms for waiting
comm.c:919: Test failed: WaitCommEvent error 87
comm.c:921: Test failed: WaitCommEvent failed with a timeout
comm.c:932: recovering after WAIT_TIMEOUT...
comm.c:938: Test failed: WaitCommEvent failed with a timeout
comm.c:943: Test failed: WaitCommEvent error 0
comm.c:944: Test failed: WaitCommEvent: expected EV_TXEMPTY, got 0
comm.c:948: WaitCommEvent for EV_TXEMPTY took 3151 ms (timeout 2133)
comm.c:949: Test failed: WaitCommEvent used 3151 ms for waiting
comm.c:1617: test_AbortWaitCts timeout 500 handle 0130
comm.c:1584:  Changing CommMask on the fly for handle 0130 after timeout 500

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Jonas Maebe]

On 05 Sep 2013, at 16:32, Dmitry Timoshkov wrote:

> The tests must pass under Wine without any additional "fixes" as they do
> currently under Windows. If you add some code to the tests which suddenly
> makes them pass under Wine - that's not a fix, Wine implementation should
> be fixed instead.

What I understood from Wolfgang's previous explanations is that there is 
currently this situation:

todo_wine
 test1();
/* test1 fails because wine has a bug. As a side-effect, bytes will be left in 
the transmission buffer under wine, but not under windows. */
test2();
/* test2 also fails under wine because of the failure of test1 that influences 
the behaviour of test2, not because the functionality tested by test2 is 
broken. This make it impossible to individually evaluate test1() and test2(). */

So either
* you add code in between that resets the state so that test2 can be run 
independently of whether test1 failed (as Wolfgang proposes), or
* you put the tests in completely different executables so they don't influence 
each other (hopefully), or
* you reorder the test so that first all the tests that currently work under 
wine are run (under the assumption that no regression will ever creep in), or
* you require that bugs in wine routines are fixed in the order that they are 
called in that test (which would be strange)


Jonas

Re: iphlpapi: Add AllocateAndGetTcpExTableFromStack() (try 6)

[Alexandre Julliard]

Ralf Habacker  writes:

> +/**
> + *  test api function AllocateAndGetTcpExTableFromStack()
> + *
> + *  Only specific tests required because the function is based
> + *  on build_tcp_table() which is covered by test_GetExtendedTcpTable()
> + */

The tests are supposed to test the behavior on Windows. The details of
the Wine implementation are irrelevant. You can't assume that the
functions share an implementation.

-- 
Alexandre Julliard
julli...@winehq.org

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:

> > > With my patches 1/2 and 2/2 I get:
> > > 
> > > fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped 0x10ee8ec):
> > > stub wine: configuration in '/home/walterw/.wine' has been updated.
> > > fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
> > > comm.c:857: Test succeeded inside todo block: WaitCommEvent failed with a
> > > timeout comm.c:881: Test succeeded inside todo block: WaitCommEvent error
> > > 997 comm.c:883: Test succeeded inside todo block: WaitCommEvent: expected
> > > EV_TXEMPTY, got 0x4 comm.c:889: Test succeeded inside todo block:
> > > WaitCommEvent used 1141 ms for waiting err:comm:set_line_control ByteSize
> > > fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0
> > > func=3 method=0) comm.c:2036: Tests skipped: interactive tests (set
> > > WINETEST_INTERACTIVE=1)
> > Successes inside of todo_wine blocks are treated as failres.
> 
> So you think I should remove the wine_todos already here?

No, the source of the failures is still there.

-- 
Dmitry.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:

> > > > > With my patches 1/2 and 2/2 I get:
> > > > > 
> > > > > fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped
> > > > > 0x10ee8ec):
> > > > > stub wine: configuration in '/home/walterw/.wine' has been updated.
> > > > > fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
> > > > > comm.c:857: Test succeeded inside todo block: WaitCommEvent failed
> > > > > with a
> > > > > timeout comm.c:881: Test succeeded inside todo block: WaitCommEvent
> > > > > error
> > > > > 997 comm.c:883: Test succeeded inside todo block: WaitCommEvent:
> > > > > expected
> > > > > EV_TXEMPTY, got 0x4 comm.c:889: Test succeeded inside todo block:
> > > > > WaitCommEvent used 1141 ms for waiting err:comm:set_line_control
> > > > > ByteSize
> > > > > fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b
> > > > > access=0
> > > > > func=3 method=0) comm.c:2036: Tests skipped: interactive tests (set
> > > > > WINETEST_INTERACTIVE=1)
> > > > 
> > > > Successes inside of todo_wine blocks are treated as failres.
> > > 
> > > So you think I should remove the wine_todos already here?
> > 
> > No, the source of the failures is still there.
> 
> What do you mean with that? The tests indeed do succeed now and there is a 
> reason they do: when you call WaitCommEvent() while the tx buffer is not 
> empty 
> yet the wine code will detect that EV_TXEMPTY correctly:

The tests must pass under Wine without any additional "fixes" as they do
currently under Windows. If you add some code to the tests which suddenly
makes them pass under Wine - that's not a fix, Wine implementation should
be fixed instead.

-- 
Dmitry.

Re: ntdll/tests: Add tests for job objects.

[Alexandre Julliard]

Andrew Cook  writes:

> diff --git a/dlls/ntdll/tests/Makefile.in b/dlls/ntdll/tests/Makefile.in
> index 10d6674..1903d10 100644
> --- a/dlls/ntdll/tests/Makefile.in
> +++ b/dlls/ntdll/tests/Makefile.in
> @@ -11,6 +11,7 @@ C_SRCS = \
>   file.c \
>   generated.c \
>   info.c \
> + job.c \

Please put this in an existing file, like om.c

> +static DWORD getProcess(PHANDLE handle) {
> +STARTUPINFO startup = {};
> +PROCESS_INFORMATION info = {};
> +
> +if(!CreateProcessA(NULL, GetCommandLine(), NULL, NULL, FALSE, 0, NULL, 
> NULL, &startup, &info)) {
> +ok(FALSE, "CreateProcess: %x\n", GetLastError());
> +return 0;
> +}
> +
> +CloseHandle(info.hThread);
> +*handle = info.hProcess;
> +
> +return info.dwProcessId;
> +}

You should use the existing winetest support for creating child
processes.

> +ok(TerminateProcess(hprocess[0], 1), "TerminateProcess: %x\n", 
> GetLastError());
> +Sleep(1000);
> +ok(TerminateProcess(hprocess[1], 2), "TerminateProcess: %x\n", 
> GetLastError());
> +Sleep(1000);

Don't add sleeps when not necessary.

> +static void is_zombie(void)
> +{
> +WCHAR val[32] = {0};
> +static const WCHAR envName[] = { 'T','E','S','T',0 };
> +static const WCHAR envVal[] = { 'J','o','b','O','b','j','e','c','t',0 };
> +
> +GetEnvironmentVariableW(envName, val, 32);
> +
> +if(lstrcmpW(envVal, val) == 0)
> +/* Wait forever, we've been created for a process handle */
> +while(1) Sleep(INFINITE);
> +
> +SetEnvironmentVariableW(envName, envVal);
> +}

That's ugly. Again, check how child processes are handled in
other tests.

-- 
Alexandre Julliard
julli...@winehq.org

Re: ws2_32: Cope with invalid protocols in WSAEnumProtocols

[Bruno Jesus]

On Thu, Sep 5, 2013 at 9:44 AM, Dmitry Timoshkov  wrote:
> Bruno Jesus <00cp...@gmail.com> wrote:
>
>> +INT validprotocols[] = { WS_IPPROTO_TCP,
>> + WS_IPPROTO_UDP,
>> + NSPROTO_IPX,
>> + NSPROTO_SPX,
>> + NSPROTO_SPXII,
>> + 0 };
>
> This should be 'static const'.

Agreed, thanks.

> --
> Dmitry.

Bruno

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 21:12:37 schrieb Dmitry Timoshkov:
> Wolfgang Walter  wrote:
> > > Does 'make test' pass without failures for you?
> > 
> > Without patch 1/2 and 2/2 I get for
> > 
> > $ ../../../tools/runtest -P wine -M kernel32.dll -T ../../.. -q -p
> > kernel32_test.exe.so comm.c
> > 
> > fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
> > err:comm:set_line_control ByteSize
> > fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0
> > func=3 method=0) comm.c:2029: Tests skipped: interactive tests (set
> > WINETEST_INTERACTIVE=1)
> The tests pass without any failures.
> 
> > With my patches 1/2 and 2/2 I get:
> > 
> > fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped 0x10ee8ec):
> > stub wine: configuration in '/home/walterw/.wine' has been updated.
> > fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
> > comm.c:857: Test succeeded inside todo block: WaitCommEvent failed with a
> > timeout comm.c:881: Test succeeded inside todo block: WaitCommEvent error
> > 997 comm.c:883: Test succeeded inside todo block: WaitCommEvent: expected
> > EV_TXEMPTY, got 0x4 comm.c:889: Test succeeded inside todo block:
> > WaitCommEvent used 1141 ms for waiting err:comm:set_line_control ByteSize
> > fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0
> > func=3 method=0) comm.c:2036: Tests skipped: interactive tests (set
> > WINETEST_INTERACTIVE=1)
> Successes inside of todo_wine blocks are treated as failres.

So you think I should remove the wine_todos already here?

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: ws2_32: Cope with invalid protocols in WSAEnumProtocols

[Dmitry Timoshkov]

Bruno Jesus <00cp...@gmail.com> wrote:

> +INT validprotocols[] = { WS_IPPROTO_TCP,
> + WS_IPPROTO_UDP,
> + NSPROTO_IPX,
> + NSPROTO_SPX,
> + NSPROTO_SPXII,
> + 0 };

This should be 'static const'.

-- 
Dmitry.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:


> > Does 'make test' pass without failures for you?
> 
> 
> Without patch 1/2 and 2/2 I get for
> 
> $ ../../../tools/runtest -P wine -M kernel32.dll -T ../../.. -q -p 
> kernel32_test.exe.so comm.c
> 
> fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
> err:comm:set_line_control ByteSize
> fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0 
> func=3 method=0)
> comm.c:2029: Tests skipped: interactive tests (set WINETEST_INTERACTIVE=1)

The tests pass without any failures.

> With my patches 1/2 and 2/2 I get:
> 
> fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped 0x10ee8ec): stub
> wine: configuration in '/home/walterw/.wine' has been updated.
> fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
> comm.c:857: Test succeeded inside todo block: WaitCommEvent failed with a 
> timeout
> comm.c:881: Test succeeded inside todo block: WaitCommEvent error 997
> comm.c:883: Test succeeded inside todo block: WaitCommEvent: expected 
> EV_TXEMPTY, got 0x4
> comm.c:889: Test succeeded inside todo block: WaitCommEvent used 1141 ms for 
> waiting
> err:comm:set_line_control ByteSize
> fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0 
> func=3 method=0)
> comm.c:2036: Tests skipped: interactive tests (set WINETEST_INTERACTIVE=1)

Successes inside of todo_wine blocks are treated as failres.

-- 
Dmitry.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 19:40:13 schrieb Dmitry Timoshkov:
> Wolfgang Walter  wrote:
> > This is not a work around.
> > 
> > todo_wine will not magically undo sending the bytes.
> > Please explain why you think that there will be now pending
> > bytes in the tx buffer if the tests fails.
> > 
> > And why it does not make the next tests fail even if wine would
> > behave correctly then.
> > 
> > res = WriteFile(hcom, tbuf, sizeof(tbuf), &bytes, NULL);
> > 
> > todo_wine
> > 
> > ok(!res, "WriteFile on an overlapped handle without ovl structure
> > should fail\n");> 
> > todo_wine
> > 
> > ok(GetLastError() == ERROR_INVALID_PARAMETER, "expected
> > ERROR_INVALID_PARAMETER, got %d\n", GetLastError());> 
> > both tests would fail with wine actually. Wine does write 17 bytes to
> > com port these bytes now sit in tx buffer and need some time sending.
> > This distorts the following tests which tests the EV_TXEMPTY behaviour:
> > 
> > Again 17 bytes are written and then the tests assume that one waits
> > for these 17 bytes (timeout value and messurement).
> > But really we wait for much more bytes being sent, up to 36.
> > 
> > So even if EV_TXEMPTY handling would work the test for it will fail with a
> > timeout.
> Does 'make test' pass without failures for you?


Without patch 1/2 and 2/2 I get for

$ ../../../tools/runtest -P wine -M kernel32.dll -T ../../.. -q -p 
kernel32_test.exe.so comm.c

fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
err:comm:set_line_control ByteSize
fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0 
func=3 method=0)
comm.c:2029: Tests skipped: interactive tests (set WINETEST_INTERACTIVE=1)

With my patches 1/2 and 2/2 I get:

fixme:iphlpapi:NotifyAddrChange (Handle 0x10ee8e0, overlapped 0x10ee8ec): stub
wine: configuration in '/home/walterw/.wine' has been updated.
fixme:comm:set_queue_size insize 1024 outsize 1024 unimplemented stub
comm.c:857: Test succeeded inside todo block: WaitCommEvent failed with a 
timeout
comm.c:881: Test succeeded inside todo block: WaitCommEvent error 997
comm.c:883: Test succeeded inside todo block: WaitCommEvent: expected 
EV_TXEMPTY, got 0x4
comm.c:889: Test succeeded inside todo block: WaitCommEvent used 1141 ms for 
waiting
err:comm:set_line_control ByteSize
fixme:ntdll:server_ioctl_file Unsupported ioctl 1b000c (device=1b access=0 
func=3 method=0)
comm.c:2036: Tests skipped: interactive tests (set WINETEST_INTERACTIVE=1)


So EV_TXEMPTY handling already works partially. 

To see that my later patches are needed you must modify the test a little bit:

- dlls/kernel32/tests/comm.c.old  2013-09-05 13:40:10.275757373 +0200
+++ dlls/kernel32/tests/comm.c  2013-09-05 13:40:06.779074398 +0200
@@ -844,6 +844,8 @@
after - before, bytes, baud);
 /* don't wait for WriteFile completion */
 
+Sleep(2000);
+
 S(U(ovl_wait)).Offset = 0;
 S(U(ovl_wait)).OffsetHigh = 0;
 ovl_wait.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

Then the tests 857 etc. again fail. We already talked about why
that is the case.

Probably I should add that as an additional test.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:

> This is not a work around.
> 
> todo_wine will not magically undo sending the bytes.
> Please explain why you think that there will be now pending
> bytes in the tx buffer if the tests fails.
> 
> And why it does not make the next tests fail even if wine would
> behave correctly then.
> 
> res = WriteFile(hcom, tbuf, sizeof(tbuf), &bytes, NULL);
> todo_wine
> ok(!res, "WriteFile on an overlapped handle without ovl structure should 
> fail\n");
> todo_wine
> ok(GetLastError() == ERROR_INVALID_PARAMETER, "expected 
> ERROR_INVALID_PARAMETER, got %d\n", GetLastError());
> 
> 
> both tests would fail with wine actually. Wine does write 17 bytes to
> com port these bytes now sit in tx buffer and need some time sending.
> This distorts the following tests which tests the EV_TXEMPTY behaviour:
> 
> Again 17 bytes are written and then the tests assume that one waits
> for these 17 bytes (timeout value and messurement).
> But really we wait for much more bytes being sent, up to 36.
> 
> So even if EV_TXEMPTY handling would work the test for it will fail with a 
> timeout.

Does 'make test' pass without failures for you?

-- 
Dmitry.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 18:55:55 schrieben Sie:
> Wolfgang Walter  wrote:
> > > > > > +if (res || (!res && GetLastError() == ERROR_IO_PENDING))
> > > > > > +/* if data has been sent: wait for termination */
> > > > > > +Sleep(timeout);
> > > > > 
> > > > > I don't see such a problem with real COM-port and serial-USB cable
> > > > > under Windows or Linux here and under testbot VMs.
> > > > 
> > > > Wine does that here (vanilla). I added this so that the NEXT test does
> > > > not
> > > > depend what wine exactly does.
> > > 
> > > When Wine behaviour differs from Windows one the test results need to be
> > > marked as todo_wine, and such places already have it.
> > 
> > I don't unterstand you. I didn't change the test case. It still above and
> > is still marked with todo.
> 
> There is no need to add any workarounds for Wine bugs, appropriate places
> already have todo_wine statements.

This is not a work around.

todo_wine will not magically undo sending the bytes.
Please explain why you think that there will be now pending
bytes in the tx buffer if the tests fails.

And why it does not make the next tests fail even if wine would
behave correctly then.

res = WriteFile(hcom, tbuf, sizeof(tbuf), &bytes, NULL);
todo_wine
ok(!res, "WriteFile on an overlapped handle without ovl structure should 
fail\n");
todo_wine
ok(GetLastError() == ERROR_INVALID_PARAMETER, "expected 
ERROR_INVALID_PARAMETER, got %d\n", GetLastError());


both tests would fail with wine actually. Wine does write 17 bytes to
com port these bytes now sit in tx buffer and need some time sending.
This distorts the following tests which tests the EV_TXEMPTY behaviour:

Again 17 bytes are written and then the tests assume that one waits
for these 17 bytes (timeout value and messurement).
But really we wait for much more bytes being sent, up to 36.

So even if EV_TXEMPTY handling would work the test for it will fail with a 
timeout.



Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 5/5] ddraw: Enumerate DXT2 and DXT4

[Stefan Dösinger]

My understanding is that we should do rgb *= a in the shader. Writing tests for 
this is on my todo list, but very close to the bottom.

(Sorry for the top post. Stupid android mail program)





Henri Verbeet  schrieb:

>On 5 September 2013 10:42, Stefan Dösinger 
>wrote:
>> Yes, we don't actually support those, but that's wined3d's problem,
>not
>> ddraw's.
>We do. The difference between DXT2/DXT3 and DXT4/DXT5 is just a
>convention wrt. storing pre-multiplied alpha or not, there's no
>difference in the actual encoding.

-- 
Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:

> > > > > +if (res || (!res && GetLastError() == ERROR_IO_PENDING))
> > > > > +/* if data has been sent: wait for termination */
> > > > > +Sleep(timeout);
> > > > 
> > > > I don't see such a problem with real COM-port and serial-USB cable
> > > > under Windows or Linux here and under testbot VMs.
> > > 
> > > Wine does that here (vanilla). I added this so that the NEXT test does not
> > > depend what wine exactly does.
> > 
> > When Wine behaviour differs from Windows one the test results need to be
> > marked as todo_wine, and such places already have it.
> 
> I don't unterstand you. I didn't change the test case. It still above and is 
> still marked with todo.

There is no need to add any workarounds for Wine bugs, appropriate places
already have todo_wine statements.

-- 
Dmitry.

Re: [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.

-- 
Dmitry.

Re: [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,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 3/5] ddraw/tests: Test DDCAPS2_TEXTUREMANAGE compatibility with other flags

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
https://newtestbot.winehq.org/JobDetails.pl?Key=2016

Your paranoid android.


=== wxppro (32 bit ddraw7) ===
ddraw7.c:3694: Test failed: Got unexpected, hr 0x8876014a, case 3.

=== wvista (32 bit ddraw7) ===
ddraw7.c:3694: Test failed: Got unexpected, hr 0x88760233, case 3.
ddraw7.c:3694: Test failed: Got unexpected, hr 0x88760233, case 8.

=== w7pro64 (32 bit ddraw7) ===
ddraw7.c:3694: Test failed: Got unexpected, hr 0x88760233, case 3.
ddraw7.c:3694: Test failed: Got unexpected, hr 0x88760233, case 8.

=== w7pro64 (64 bit ddraw7) ===
ddraw7.c:3694: Test failed: Got unexpected, hr 0x80004002, case 2.
ddraw7.c:3694: Test failed: Got unexpected, hr 0x88760233, case 3.
ddraw7.c:3694: Test failed: Got unexpected, hr 0x80004002, case 4.
ddraw7.c:3694: Test failed: Got unexpected, hr 0x88760233, case 8.

=== wxppro (32 bit ddraw4) ===
ddraw4.c:3880: Test failed: Got unexpected, hr 0x8876014a, case 3.

=== wvista (32 bit ddraw4) ===
ddraw4.c:3880: Test failed: Got unexpected, hr 0x88760233, case 3.
ddraw4.c:3880: Test failed: Got unexpected, hr 0x88760233, case 8.

=== w7pro64 (32 bit ddraw4) ===
ddraw4.c:3880: Test failed: Got unexpected, hr 0x88760233, case 3.
ddraw4.c:3880: Test failed: Got unexpected, hr 0x88760233, case 8.

=== w7pro64 (64 bit ddraw4) ===
ddraw4.c:3880: Test failed: Got unexpected, hr 0x88760233, case 3.
ddraw4.c:3880: Test failed: Got unexpected, hr 0x88760233, case 8.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 17:54:58 schrieb Dmitry Timoshkov:
> Wolfgang Walter  wrote:
> > > > +if (res || (!res && GetLastError() == ERROR_IO_PENDING))
> > > > +/* if data has been sent: wait for termination */
> > > > +Sleep(timeout);
> > > 
> > > I don't see such a problem with real COM-port and serial-USB cable
> > > under Windows or Linux here and under testbot VMs.
> > 
> > Wine does that here (vanilla). I added this so that the NEXT test does not
> > depend what wine exactly does.
> 
> When Wine behaviour differs from Windows one the test results need to be
> marked as todo_wine, and such places already have it.

I don't unterstand you. I didn't change the test case. It still above and is 
still marked with todo.

This change simply ensures that the next test is not distorted. If wine does 
not pass the test

"WriteFile on an overlapped handle without ovl structure should fail"

but instead sends the bytes we should wait long enough before starting the 
next test so that there are no remaining bytes in the tx buffer. Otherwise the 
next test will not test sending 17 bytes but much more.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [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.

-- 
Dmitry.

Re: [PATCH 3/5] ddraw/tests: Test DDCAPS2_TEXTUREMANAGE compatibility with other flags

[Stefan Dösinger]

Oops, will resend later.



Henri Verbeet  schrieb:
>On 5 September 2013 10:42, Stefan Dösinger 
>wrote:
>> +if(DDSD->ddsCaps.dwCaps2 & DDSCAPS2_TEXTUREMANAGE)
>Minor formatting error here.

--
Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

Re: [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.
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 1/5] use a correct timeout in test_waittxempty

[Wolfgang Walter]

Am Donnerstag, 5. September 2013, 17:51:18 schrieb Dmitry Timoshkov:
> Wolfgang Walter  wrote:
> > > > When we send 17 bytes with 150 baud, no parity, one stop bit then
> > > > we need to wait at least 17*10/150 seconds > 1000 ms
> > > 
> > > Under Windows with both real COM-port and USB-serial cable this test
> > > takes
> > > no longer than 890 ms, usually it's even shorter like 875 ms. Testbot
> > > VMs
> > > also don't fail this test. If under Wine it takes > 1000 ms for you then
> > > probably there is a bug somewhere.
> > 
> > How can that be? If you send 17 bytes which expands to 170 raw bits and
> > you
> > only sent 150 bits/s?
> > 
> > Maybe you are using a virtual machine or your com port does not support
> > 150
> > baud and chooses a different speed. Or you use a UART with a large buffer
> > and which already signals TX-EMPTY even if it is still sending from an
> > internal buffer. But one should not rely on that.
> 
> I'm testing under 64-bit Windows 7 on real hardware.

Then it is your UART or it's driver. As I sad: you need 1133 ms to transmit 17 
bytes and I think one should not wait less. Otherwise one depends on hardware, 
driver internals (linux as windows) and scheduling. One should even wait 
longer (that is the reason I add TIMEOUT).

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 5/5] ddraw: Enumerate DXT2 and DXT4

[Henri Verbeet]

On 5 September 2013 10:42, Stefan Dösinger  wrote:
> Yes, we don't actually support those, but that's wined3d's problem, not
> ddraw's.
We do. The difference between DXT2/DXT3 and DXT4/DXT5 is just a
convention wrt. storing pre-multiplied alpha or not, there's no
difference in the actual encoding.

Re: [PATCH 3/5] ddraw/tests: Test DDCAPS2_TEXTUREMANAGE compatibility with other flags

[Henri Verbeet]

On 5 September 2013 10:42, Stefan Dösinger  wrote:
> +if(DDSD->ddsCaps.dwCaps2 & DDSCAPS2_TEXTUREMANAGE)
Minor formatting error here.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:

> > > +if (res || (!res && GetLastError() == ERROR_IO_PENDING))
> > > +/* if data has been sent: wait for termination */
> > > +Sleep(timeout);
> > 
> > I don't see such a problem with real COM-port and serial-USB cable
> > under Windows or Linux here and under testbot VMs.
> 
> Wine does that here (vanilla). I added this so that the NEXT test does not 
> depend what wine exactly does.

When Wine behaviour differs from Windows one the test results need to be
marked as todo_wine, and such places already have it.

-- 
Dmitry.

Re: [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.

-- 
Dmitry.

Re: [PATCH 1/5] use a correct timeout in test_waittxempty

[Dmitry Timoshkov]

Wolfgang Walter  wrote:

> > > When we send 17 bytes with 150 baud, no parity, one stop bit then
> > > we need to wait at least 17*10/150 seconds > 1000 ms
> > 
> > Under Windows with both real COM-port and USB-serial cable this test takes
> > no longer than 890 ms, usually it's even shorter like 875 ms. Testbot VMs
> > also don't fail this test. If under Wine it takes > 1000 ms for you then
> > probably there is a bug somewhere.
> 
> How can that be? If you send 17 bytes which expands to 170 raw bits and you 
> only sent 150 bits/s?
> 
> Maybe you are using a virtual machine or your com port does not support 150 
> baud and chooses a different speed. Or you use a UART with a large buffer and 
> which already signals TX-EMPTY even if it is still sending from an internal 
> buffer. But one should not rely on that.

I'm testing under 64-bit Windows 7 on real hardware.

-- 
Dmitry.

Re: [PATCH 2/5] wait until all data from earlier test has been written in test_waittxempty

[Wolfgang Walter]

On Thursday 05 September 2013 10:37:22 you wrote:
> Wolfgang Walter  wrote:
> > +if (res || (!res && GetLastError() == ERROR_IO_PENDING))
> > +/* if data has been sent: wait for termination */
> > +Sleep(timeout);
> 
> I don't see such a problem with real COM-port and serial-USB cable
> under Windows or Linux here and under testbot VMs.

Wine does that here (vanilla). I added this so that the NEXT test does not 
depend what wine exactly does.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Recht
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [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,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Recht
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: [PATCH 1/5] use a correct timeout in test_waittxempty

[Wolfgang Walter]

On Thursday 05 September 2013 10:32:15 Dmitry Timoshkov wrote:
> Wolfgang Walter  wrote:
> > When we send 17 bytes with 150 baud, no parity, one stop bit then
> > we need to wait at least 17*10/150 seconds > 1000 ms
> 
> Under Windows with both real COM-port and USB-serial cable this test takes
> no longer than 890 ms, usually it's even shorter like 875 ms. Testbot VMs
> also don't fail this test. If under Wine it takes > 1000 ms for you then
> probably there is a bug somewhere.

How can that be? If you send 17 bytes which expands to 170 raw bits and you 
only sent 150 bits/s?

Maybe you are using a virtual machine or your com port does not support 150 
baud and chooses a different speed. Or you use a UART with a large buffer and 
which already signals TX-EMPTY even if it is still sending from an internal 
buffer. But one should not rely on that.

I used a real com-port, by the way, and had a com-port-analyzer adapted to it. 
It exactly sends 150 bits per second which means 15 data-bytes per second.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Recht
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Re: D3D command stream patches for testing

[Stefan Dösinger]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Am 2013-09-04 20:11, schrieb Prot Secret:
> Hi, I am interested in your work, but after patching wine171 of
> this patch and enabled csmt, i have seen this in SC2 
> http://postimg.org/gallery/1w8y6jbg I use nvidia GTX 680 with
> proprietary i386 nvidia driver 319.49.
> 
> Where I could be wrong, or what else can be set that would be
> normalized image using the patch?
Not your fault, there is a race condition in the surface code that was
recently introduced. I will post an updated patchset in the next few days.

Stefan


-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.20 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSKDfBAAoJEN0/YqbEcdMw8WgP/RoPby9sd8BrVCeCU6MpjNYf
5belZtZir82BWGbR5EyO0bUq6gjVUaIMRK1daeoMLbv3TXKTGc8H9NdbZNZKldjl
b2WIXHWTNC0wS6xuEeNNasCuX9TvAzpMUOuLtdUBzaGSYqpW2hHW4h6weTL/ytfO
moQrbL0C3KZb/K6nPJiwDAbFR+gHk8C+aog0NUFJDj/kvRXHif+6yMyDzFsK8FX0
U5pnVa9/eyoAV2Qc42gruD3PP/VLulua9xk0nB4sDp+hBlqYtOv8IULY0D0J7nTd
XwEvDczy4tf60lhoivcIajDxP2CvTAbQSlM+nbPN7kJua6S/5CRlHzaI+cyWiWII
za6I3UKcMJP6J8aPg7Rbt/lVhN4xGwYQrSih5cZg3iIzFvuXPbDwg1ZjSzdyK6ya
xzd3t2BrOt4yrq3scGw3BcbMOKCvnq5/PiNgw2HGhnkISIAzv1vHcizMQDTHmu97
znlRJ/ynYxXuOinSBqM7Y44mKXx+IwgQBQjBI1TZLHIJYB7MN9+YIuDT7GvGY8r0
20cOS9pkdaYVXNyjRZ8bQkoQ14lCnsHaZ1Kre3fQzhNCuWRcCxq0jKFORnFZ+A+w
8axUENqD1frASvVtFQ0uKMiRXRmk5bIJZ1uJscupAGRWQbV4yflEiQ9eTODH+1Uc
NMTqvTumhE5/CdB3mJmF
=dIov
-END PGP SIGNATURE-