April 2016 - Wine-Devel - list.winehq.org

Re: [5/5] wininet/tests: Add more tests for INTERNET_INVALID_PORT_NUMBER. (resend)

by 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://testbot.winehq.org/JobDetails.pl?Key=22619 Your paranoid android. === w1064 (32 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2406: Test failed: count was wrong http.c:2407: Test failed: http data wrong, got: Garbage: Header http.c:3070: Test failed: HttpAddRequestHeaders succeeded http.c:3071: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3076: Test failed: code = 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 === w1064 (64 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2406: Test failed: count was wrong http.c:2407: Test failed: http data wrong, got: Garbage: Header http.c:3070: Test failed: HttpAddRequestHeaders succeeded http.c:3071: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3076: Test failed: code = 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1

April 26, 2016

Re: [4/5] wininet/tests: Add tests for asynchronous InternetReadFile. (v3)

by 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://testbot.winehq.org/JobDetails.pl?Key=22618 Your paranoid android. === w1064 (32 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2406: Test failed: count was wrong http.c:2407: Test failed: http data wrong, got: Garbage: Header http.c:3070: Test failed: HttpAddRequestHeaders succeeded http.c:3071: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3076: Test failed: code = 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 === w1064 (64 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2406: Test failed: count was wrong http.c:2407: Test failed: http data wrong, got: Garbage: Header http.c:3070: Test failed: HttpAddRequestHeaders succeeded http.c:3071: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3076: Test failed: code = 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1

April 26, 2016

Re: [2/5] wininet/tests: Add tests for asynchronous InternetReadFileEx. (v3)

by 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://testbot.winehq.org/JobDetails.pl?Key=22617 Your paranoid android. === w1064 (32 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2406: Test failed: count was wrong http.c:2407: Test failed: http data wrong, got: Garbage: Header http.c:3070: Test failed: HttpAddRequestHeaders succeeded http.c:3071: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3076: Test failed: code = 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 === w1064 (64 bit http) === http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1 http.c:1762: Test failed: Expected 2, got 4 http.c:1763: Test failed: Expected CRLF, got ' http.c:2406: Test failed: count was wrong http.c:2407: Test failed: http data wrong, got: Garbage: Header http.c:3070: Test failed: HttpAddRequestHeaders succeeded http.c:3071: Test failed: Expected error ERROR_HTTP_HEADER_NOT_FOUND, got 0 http.c:3076: Test failed: code = 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:3076: Test failed: unexpected status code 200, expected 400 http.c:523: Test failed: HTTP_QUERY_RAW_HEADERS: expected length 0, but got 1

April 26, 2016

Re: [2/6] wininet/tests: Add tests for asynchronous InternetReadFileEx.

by Sebastian Lackner

On 26.04.2016 23:13, Jacek Caban wrote: > On 04/26/16 23:00, Sebastian Lackner wrote: >> The idea of this additional check was to ensure that the request cannot be >> fulfilled with the current buffer contents. Otherwise its not completely >> clear that async handler is really waiting for new data, as it should be. >> If you prefer, I can also remove this test again or set the timeout to 0 to >> avoid blocking. As the tests show, this part is already working fine. ;) > > Yes, please. Both timeout 0 and removing the test is fine with me. > > BTW, other patches look good to me. > > Thanks, > Jacek > Thanks, I've sent a new version.

April 26, 2016

Re: [PATCH v2] kernel32: Use setprogname(3) to set the program name if available.

by Theodore Dubois

Don’t know why I didn’t think of that. I’m going to give it a try and see if there’s any reason I can’t get it to work. ~Theodore > On Apr 26, 2016, at 1:38 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > > On 26.04.2016 22:35, Theodore Dubois wrote: >> I have both a custom wine loader and a hacked version of Wine, so I just reverted the patch. >> >> It would be really nice to have this work for when I’m not using a custom loader. Maybe you could check if argv is in the malloc heap? Or maybe I could somehow indicate when calling wine_init that the argv is malloced? >> >> ~Theodore >> > > May I ask why you are not just allocating the argv array in a way compatible to the OS? > When you have full control in your custom loader, this should be trivial and the easiest > way to fix it. > > Regards, > Sebastian > > >

April 26, 2016

Re: [2/6] wininet/tests: Add tests for asynchronous InternetReadFileEx.

by Jacek Caban

On 04/26/16 23:00, Sebastian Lackner wrote: > The idea of this additional check was to ensure that the request cannot be > fulfilled with the current buffer contents. Otherwise its not completely > clear that async handler is really waiting for new data, as it should be. > If you prefer, I can also remove this test again or set the timeout to 0 to > avoid blocking. As the tests show, this part is already working fine. ;) Yes, please. Both timeout 0 and removing the test is fine with me. BTW, other patches look good to me. Thanks, Jacek

April 26, 2016

Re: [6/6] wininet: Handle INTERNET_INVALID_PORT_NUMBER in HttpOpenRequest.

by Jacek Caban

On 04/26/16 16:13, Sebastian Lackner wrote: > On 26.04.2016 14:44, Jacek Caban wrote: >> Note that get_server already has code to deal with >> INTERNET_INVALID_PORT_NUMBER, so it would be enough to handle https case >> there. However, given that all other callers have valid port numbers, I >> agree that it would be cleaner to do that in HTTP_HttpOpenRequestW >> instead. Given that your moving the logic around, please remove no >> longer needed code there. >> >> Thanks, >> Jacek >> >> > I will resend a fix for this after the first bunch of patches is accepted. > Removing the code from get_server() would probably not be sufficient, it > looks like HTTP_DealWithProxy() might also depend on it. In HTTP_DealWithProxy it should be probably removed as well, because InternetCrackUrl should take care of that. Jacek

April 26, 2016

Re: [2/6] wininet/tests: Add tests for asynchronous InternetReadFileEx.

by Sebastian Lackner

On 26.04.2016 22:47, Jacek Caban wrote: > Hi Sebastian, > > On 04/26/16 16:11, Sebastian Lackner wrote: >> On 26.04.2016 14:30, Jacek Caban wrote: >>> It's a nice way to force async read, but when you add tests with >>> predictable reads it would be nice to take more advantage of that. What >>> do you think about: >>> >>> - call InternetReadFileEx and check that it's done sync >>> - call InternetReadFileEx and check that it's done async >>> - set conn_wait_event, wait completion and make sure we read all >>> remaining data >>> - call InternetReadFileEx and make sure it synchronously hits the end of >>> data >>> >>> Thanks, >>> Jacek >>> >>> >> I don't think its necessary to restructure the whole code, unrolling the >> loop would require a lot of code duplication because Wine still behaves >> different in many aspects, and even on Windows it depends on things like >> buffer sizes. > > I expected Wine to be closer to Windows in this case. I experimented a > bit with your tests and I can see that those are less predictable than I > thought. Yes, thats also what I noticed when I played around with it. We can probably make the tests more precise at some later point. > You'd see Wine behaviour by using IRF_NO_WAIT, so handling > flags 0 is a problem in Wine, but not the only one. Anyway, those are > unrelated to your changes, so it's fine as is. > > >> I've added more tests (some as todo_wine) for the things you >> pointed out below though. Are you fine with the new patch, or any other >> cases you would like to have tested? > > Thanks for adding them. Just a few comments: > > + if (!pending_reads++) > + { > + res = WaitForSingleObject( hCompleteEvent, 200 ); > + ok( res == WAIT_TIMEOUT, "expected WAIT_TIMEOUT, got %u\n", res ); > + SetEvent( conn_wait_event ); > + } > > > I'm not sure what is this for and I'd like to avoid adding expected > timeouts is possible. Checking last error ERROR_IO_PENDING seems enough > to consider it being done asynchronously. Sorry if you misunderstood my > comment about checking it for being async. The idea of this additional check was to ensure that the request cannot be fulfilled with the current buffer contents. Otherwise its not completely clear that async handler is really waiting for new data, as it should be. If you prefer, I can also remove this test again or set the timeout to 0 to avoid blocking. As the tests show, this part is already working fine. ;) > > + todo_wine_if( ib.dwBufferLength == 0 ) > + ok( ib.dwBufferLength != 0, "expected ib.dwBufferLength != 0\n" ); > > > todo_wine_if(pending_reads > 1) should do the trick here and won't > completely disable the test on Wine. Okay, I will change that. > > Same comments apply to InternetReadFile tests. > > Thanks, > Jacek >

April 26, 2016

Re: [2/6] wininet/tests: Add tests for asynchronous InternetReadFileEx.

by Jacek Caban

Hi Sebastian, On 04/26/16 16:11, Sebastian Lackner wrote: > On 26.04.2016 14:30, Jacek Caban wrote: >> It's a nice way to force async read, but when you add tests with >> predictable reads it would be nice to take more advantage of that. What >> do you think about: >> >> - call InternetReadFileEx and check that it's done sync >> - call InternetReadFileEx and check that it's done async >> - set conn_wait_event, wait completion and make sure we read all >> remaining data >> - call InternetReadFileEx and make sure it synchronously hits the end of >> data >> >> Thanks, >> Jacek >> >> > I don't think its necessary to restructure the whole code, unrolling the > loop would require a lot of code duplication because Wine still behaves > different in many aspects, and even on Windows it depends on things like > buffer sizes. I expected Wine to be closer to Windows in this case. I experimented a bit with your tests and I can see that those are less predictable than I thought. You'd see Wine behaviour by using IRF_NO_WAIT, so handling flags 0 is a problem in Wine, but not the only one. Anyway, those are unrelated to your changes, so it's fine as is. > I've added more tests (some as todo_wine) for the things you > pointed out below though. Are you fine with the new patch, or any other > cases you would like to have tested? Thanks for adding them. Just a few comments: + if (!pending_reads++) + { + res = WaitForSingleObject( hCompleteEvent, 200 ); + ok( res == WAIT_TIMEOUT, "expected WAIT_TIMEOUT, got %u\n", res ); + SetEvent( conn_wait_event ); + } I'm not sure what is this for and I'd like to avoid adding expected timeouts is possible. Checking last error ERROR_IO_PENDING seems enough to consider it being done asynchronously. Sorry if you misunderstood my comment about checking it for being async. + todo_wine_if( ib.dwBufferLength == 0 ) + ok( ib.dwBufferLength != 0, "expected ib.dwBufferLength != 0\n" ); todo_wine_if(pending_reads > 1) should do the trick here and won't completely disable the test on Wine. Same comments apply to InternetReadFile tests. Thanks, Jacek

April 26, 2016

Re: [PATCH v2] kernel32: Use setprogname(3) to set the program name if available.

by Sebastian Lackner

On 26.04.2016 22:35, Theodore Dubois wrote: > I have both a custom wine loader and a hacked version of Wine, so I just reverted the patch. > > It would be really nice to have this work for when I’m not using a custom loader. Maybe you could check if argv is in the malloc heap? Or maybe I could somehow indicate when calling wine_init that the argv is malloced? > > ~Theodore > May I ask why you are not just allocating the argv array in a way compatible to the OS? When you have full control in your custom loader, this should be trivial and the easiest way to fix it. Regards, Sebastian

April 26, 2016