January 2015 - Wine-Devel - list.winehq.org

Re: winhttp/tests: Use a wait event instead of an unconditional sleep in the server thread.

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=11019 Your paranoid android. === w8 (32 bit winhttp) === winhttp.c:2042: Test failed: failed to receive response 12152 winhttp.c:2047: Test failed: request failed unexpectedly 327681

Jan. 5, 2015

Re: dplayx: Cache the results of DirectPlayEnumerateAW()

by Bruno Jesus

On Mon, Jan 5, 2015 at 7:19 AM, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hi Bruno, Hi, Stefan. Thanks for the review. > Some tests for this behavior would be useful. Also consider adding tests that check if DirectPlayEnumerate always enumerates the same pointers. I'll try. > Am 2015-01-04 um 23:05 schrieb Bruno Jesus: >> + static struct _data >> + { >> + GUID guid; >> + WCHAR descriptionW[256]; >> + char descriptionA[256]; >> + } *providers_cache; > You don't have to give this structure a name. (i.e., remove "_data"). OK. >> + /* Some applications require that the data returned through the callback persist after the callbacks >> + * are called, so we buffer the information first and do the callbacks later. We will always rewrite >> + * the buffer as some applications may change it (on purpose or due to bugs). >> + */ > Please name the application. 2 years from now nobody will remember it, and a search through wine-patches might not be successful. Makes sense. >> + HeapFree(GetProcessHeap(), 0, providers_cache); >> + providers_cache = HeapAlloc(GetProcessHeap(), 0, sizeof(*providers_cache) * dwIndex); >> + if (!providers_cache) >> + { >> + ERR(": failed to alloc required memory.\n"); >> + return DPERR_EXCEPTION; >> + } > Does the registry data change during runtime? I think it is created during prefix creation and then never touched again. I don't know, I'm just worried that the application may destroy the data, that's why I Always re-read it. >> + if (sizeOfDescription > max_sizeOfDescriptionW) >> + { >> + HeapFree(GetProcessHeap(), 0, descriptionW); >> + max_sizeOfDescriptionW = sizeOfDescription; >> + } >> + descriptionW = HeapAlloc(GetProcessHeap(), 0, sizeOfDescription); > This can leak descriptionW. You're also hard-coding a size limit of 256 in providers_cache. One of the two isn't right, most likely the hard-coded limit. This is the current code, I didn't change it. The patch may seem large but it's an indentation difference. >> + RegQueryValueExW(hkServiceProvider, descW, >> + NULL, NULL, (LPBYTE) descriptionW, &sizeOfDescription); >> + >> + lstrcpynW(providers_cache[dwIndex].descriptionW, descriptionW, sizeof(providers_cache[0].descriptionW)); >> + WideCharToMultiByte(CP_ACP, 0, providers_cache[dwIndex].descriptionW, -1, providers_cache[dwIndex].descriptionA, >> + sizeof(providers_cache[0].descriptionA), NULL, NULL); > The old code queries two different values, "DescriptionA" and "DescriptionW". Your code queries "DescriptionW" and converts the content to ASCII. Do DescriptionA and DescriptionW always contain the same content? I assumed it could be the same because in English and Portuguese systems it is, but thinking more properly it may make a difference for non-roman writing languages, I will cache both. Best Regards, Bruno

Jan. 5, 2015

Re: dplayx: Cache the results of DirectPlayEnumerateAW()

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Bruno, Some tests for this behavior would be useful. Also consider adding tests that check if DirectPlayEnumerate always enumerates the same pointers. Am 2015-01-04 um 23:05 schrieb Bruno Jesus: > + static struct _data > + { > + GUID guid; > + WCHAR descriptionW[256]; > + char descriptionA[256]; > + } *providers_cache; You don't have to give this structure a name. (i.e., remove "_data"). > + /* Some applications require that the data returned through the callback persist after the callbacks > + * are called, so we buffer the information first and do the callbacks later. We will always rewrite > + * the buffer as some applications may change it (on purpose or due to bugs). > + */ Please name the application. 2 years from now nobody will remember it, and a search through wine-patches might not be successful. > + HeapFree(GetProcessHeap(), 0, providers_cache); > + providers_cache = HeapAlloc(GetProcessHeap(), 0, sizeof(*providers_cache) * dwIndex); > + if (!providers_cache) > + { > + ERR(": failed to alloc required memory.\n"); > + return DPERR_EXCEPTION; > + } Does the registry data change during runtime? I think it is created during prefix creation and then never touched again. > + if (sizeOfDescription > max_sizeOfDescriptionW) > + { > + HeapFree(GetProcessHeap(), 0, descriptionW); > + max_sizeOfDescriptionW = sizeOfDescription; > + } > + descriptionW = HeapAlloc(GetProcessHeap(), 0, sizeOfDescription); This can leak descriptionW. You're also hard-coding a size limit of 256 in providers_cache. One of the two isn't right, most likely the hard-coded limit. > + RegQueryValueExW(hkServiceProvider, descW, > + NULL, NULL, (LPBYTE) descriptionW, &sizeOfDescription); > + > + lstrcpynW(providers_cache[dwIndex].descriptionW, descriptionW, sizeof(providers_cache[0].descriptionW)); > + WideCharToMultiByte(CP_ACP, 0, providers_cache[dwIndex].descriptionW, -1, providers_cache[dwIndex].descriptionA, > + sizeof(providers_cache[0].descriptionA), NULL, NULL); The old code queries two different values, "DescriptionA" and "DescriptionW". Your code queries "DescriptionW" and converts the content to ASCII. Do DescriptionA and DescriptionW always contain the same content? -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUqlckAAoJEN0/YqbEcdMwPYcP/1zJL5U+YlDNVLsjHaiZQClj x0gyVMUcAB/0IH5QA1l37o0OChMgPg+xjkKuHfV9yx1ESFiulbm6bb1Qc1MfHmSO KB0hswxd1G9LVw8VUcbfCMmS3LK4JQl/Ha8Zj2RcFpcU+xIRc+p0LY+bg0QJyML2 i0XYSomKubZoKDeW1ZW8oAGXHc67kT2kBVXoZ9jCThjob8Lzy0bsZaKqEGiEjm5v UqGCjfv7wLsd3egEAA6WBa/sxmV5pGUqPbYe0OLNxExtOOHmZE7u0Ti9bAdGLZLE FnEB46IW6ECgMbdizIh9yZU/1pNB1AjVXKC1nhJevPPA4Bbln+4OkOrjKuYSQyqq lITarHscB9wnDdhXc3iwvi1vOB1B0v0X7CvTSAc+xBkx7gjK8/pyRSShcm6ho7yj 7UeKJuTqZ/+WRoMX2ariim7vzSzedWw+P1V0vcO/J5bA6icWpAxoivihSM7OzXFT lFYAr98BwlwVljr7QYKMtC1CkmBHIXIaaIoV7c6miTounpwXGvI4BdXvOt679jTV bKfMo7p5rx9n2K2cro7/J/g0bPcIX6PAg0ay9g+jD7XzhWkxQTjzHQF79uko3XG7 U3Y3ApC1dRwzJ7PzDIiS3F0t1Bk1+PcaMK269cOISni9odiODz1I7uzQFsVJE0S8 H9a8YJIdWhzvjszLKn44 =76/t -----END PGP SIGNATURE-----

Jan. 5, 2015

Re: kernel32: Implement Get{Time,Date}FormatEx

by Nikolay Sivov

On 04.01.2015 10:11, Seán de Búrca wrote: > Hi Nikolay, > > On 12/29/2014 20:12, Nikolay Sivov wrote: > >>On 30.12.2014 2:06, Sean Burke wrote: >>>/ Provides an implementation of time and date format functions added in > />>/ Vista sufficient to allow some basic applications requiring them to run. > />>/ Includes necessary tests and header defs. > />>/ > />>/ > />>/ Sean Burke > />>/ > />>/ > />>/ > / >>Hi, Sean. > >>As you can see tests are failing on win2000 and winxp because these >>calls are not available on these platforms. You should use >>GetProcAddress() to fix that. > >>>/ +@ stdcall GetDateFormatEx(ptr long ptr wstr ptr long ptr) > / >>First argument should be 'wstr' as well. > >>>/ +@ stdcall GetTimeFormatEx(ptr long ptr wstr ptr long) > / >>Same. > >>>/ +INT WINAPI GetDateFormatEx(LPCWSTR lpLocaleName, DWORD dwFlags, > />>/ + const SYSTEMTIME* lpDate, LPCWSTR lpFormat, > />>/ + LPWSTR lpDateStr, INT cchDate, LPCWSTR lpCalendar) > / >>Please use cleaner argument names, like 'localename', 'flags', 'date', >>'format', etc. So lowercase, no prefix. > > The rest of the file uses argument names directly from the MSDN documentation. I'm inclined to follow this convention as I think it provides a clear and direct connection to the API as documented. However, I am unfamiliar with current wine conventions, so if you still feel this is a worthwhile change, let me know. It doesn't have to directly match MSDN names, in fact it's good to be different in such small things. But if you prefer such naming it's not a stopper for this patch. > >>>/ + lcid = LocaleNameToLCID(lpLocaleName, 0); > />>/ + > />>/ + return NLS_GetDateTimeFormatW(lcid, dwFlags | DATE_DATEVARSONLY, lpDate, > />>/ + lpFormat, lpDateStr, cchDate); > / >>No real need for temporary 'lcid' variable. > > > Sean Burke >

Jan. 4, 2015

Re: kernel32: Implement Get{Time,Date}FormatEx

by Seán de Búrca

Hi Nikolay, On 12/29/2014 20:12, Nikolay Sivov wrote: >On 30.12.2014 2:06, Sean Burke wrote: >>/ Provides an implementation of time and date format functions added in />>/ Vista sufficient to allow some basic applications requiring them to run. />>/ Includes necessary tests and header defs. />>/ />>/ />>/ Sean Burke />>/ />>/ />>/ / >Hi, Sean. >As you can see tests are failing on win2000 and winxp because these >calls are not available on these platforms. You should use >GetProcAddress() to fix that. >>/ +@ stdcall GetDateFormatEx(ptr long ptr wstr ptr long ptr) / >First argument should be 'wstr' as well. >>/ +@ stdcall GetTimeFormatEx(ptr long ptr wstr ptr long) / >Same. >>/ +INT WINAPI GetDateFormatEx(LPCWSTR lpLocaleName, DWORD dwFlags, />>/ + const SYSTEMTIME* lpDate, LPCWSTR lpFormat, />>/ + LPWSTR lpDateStr, INT cchDate, LPCWSTR lpCalendar) / >Please use cleaner argument names, like 'localename', 'flags', 'date', >'format', etc. So lowercase, no prefix. The rest of the file uses argument names directly from the MSDN documentation. I'm inclined to follow this convention as I think it provides a clear and direct connection to the API as documented. However, I am unfamiliar with current wine conventions, so if you still feel this is a worthwhile change, let me know. >>/ + lcid = LocaleNameToLCID(lpLocaleName, 0); />>/ + />>/ + return NLS_GetDateTimeFormatW(lcid, dwFlags | DATE_DATEVARSONLY, lpDate, />>/ + lpFormat, lpDateStr, cchDate); / >No real need for temporary 'lcid' variable. Sean Burke

Jan. 4, 2015

Re: secur32: Add support for ECDHE algorithms using gnutls

by Seán de Búrca

Hi Jacek, On 12/30/14 08:34, Jacek Caban wrote: >Hi Sean, >On 12/30/14 00:08, Sean Burke wrote: >>/ diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c />>/ index 638b3c5..810c598 100644 />>/ --- a/dlls/secur32/schannel_gnutls.c />>/ +++ b/dlls/secur32/schannel_gnutls.c />>/ @@ -328,6 +328,8 @@ static ALG_ID schannel_get_kx_algid(gnutls_kx_algorithm_t kx) />>/ switch (kx) />>/ { />>/ case GNUTLS_KX_RSA: return CALG_RSA_KEYX; />>/ + case GNUTLS_KX_ECDHE_ECDSA: / >Shouldn't this case use CALG_ECDSA? It's possible. I based this on dlls/secur32/schannel_macosx.c:501, where _ECDHE_ECDSA returns CALG_ECDH. If this is incorrect, please let me know. >>/ + case GNUTLS_KX_ECDHE_RSA: return CALG_ECDH; />>/ case GNUTLS_KX_DHE_DSS: />>/ case GNUTLS_KX_DHE_RSA: return CALG_DH_EPHEM; />>/ default: / >Cheers, >Jacek Sean Burke

Jan. 4, 2015

Re: Unicode Support in wine-devel

by Akira Nakagawa

>I can see those working fine on this mailing list both through my mail client and through both of the online mailing list browsers ( https://www.winehq.org/pipermail/wine-devel/ && http://news.gmane.org/gmane.comp.emulators.wine.devel ) So, are You sure there is an actual problem? Probably the encoding of the mail sent by wine-devel-digest is not formatted as Unicode. It doesn't problem of server, but the mail. All chinese characters are viewed as ??????? in my gmail. It is a SMTP's problem,isn't it?

Jan. 2, 2015

Re: [PATCH] winealsa: support up to 6 channels

by Andrew Eikum

On Wed, Dec 31, 2014 at 02:01:36PM -0800, Chris Robinson wrote: > On 12/31/2014 01:34 PM, Mark Harmstone wrote: > >Needed for surround sound. > > ALSA uses a different channel mapping for 5.1 than Windows. Windows wants: > front-left, front-right, front-center, lfe, back-left*, back-right* > while ALSA wants: > front-left, front-right, back-left*, back-right*, front-center, lfe > > So you can't just pass the samples through without swapping them. > I believe winealsa already handles this, though I'd welcome a review. See map_channels() in <dlls/winealsa.drv/mmdevdrv.c>: http://source.winehq.org/git/wine.git/blob/6bab17313918ec21b196c7e465500f4c… I'd like to use the new ALSA channel mapping API at some point, but in the meantime I think it's handled well enough. Andrew

Jan. 2, 2015

Re: [PATCH] msvcr120: Fixed bugs in Concurrency::critical_section.

by Piotr Caban

Hi, the tests are failing on both wine and windows with following message: msvcr120.c:303: Test failed: execution timed out, critical section probably not functioning correctly msvcr120: 155 tests executed (0 marked as todo, 1 failure), 0 skipped. It takes ~14 seconds to run the tests here. You also left "#include "stdio.h" line in the tests. The change in critical_section class implementation looks good for me. Cheers, Piotr

Jan. 2, 2015