November 2014 - Wine-devel - list.winehq.org

Re: [PATCH] winealsa: fix input sound distortion and heap corruption
by Andrew Eikum 07 Nov '14

07 Nov '14

On Fri, Nov 07, 2014 at 04:06:55AM +0000, Mark Harmstone wrote: > At present, winealsa attempts to call map_channels when a capture > device is opened. If a microphone can be opened in either stereo or > mono mode, and mono is requested, Wine will still return stereo > data. Not only does this result in distorted in audio, it also very > frequently results in heap corruption when snd_pcm_readi overruns > the buffer allocated by the program. > Thanks for fixing this. The patch doesn't apply because it got mangled in transit: http://source.winehq.org/patches/data/107529 The idea look OK to me, but don't we want to disable channel remapping for _all_ capture devices? Your patch only disables it if the application is using WAVE_FORMAT_EXTENSIBLE I don't think we can do capture channel remapping reliably without switching to the new ALSA channel mapping API. So it makes sense to disable it entirely for now. Andrew > I don't have a copy of World of Warcraft to test this on, but this > may also fix bug 31684. It's the same symptoms, at least. > > --- > dlls/winealsa.drv/mmdevdrv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c > index a4c02bf..91fba2f 100644 > --- a/dlls/winealsa.drv/mmdevdrv.c > +++ b/dlls/winealsa.drv/mmdevdrv.c > @@ -1164,7 +1164,7 @@ static DWORD get_channel_mask(unsigned int channels) > > static HRESULT map_channels(ACImpl *This, const WAVEFORMATEX *fmt) > { > - if(fmt->wFormatTag == WAVE_FORMAT_EXTENSIBLE || fmt->nChannels > 2){ > + if( (This->dataflow != eCapture && fmt->wFormatTag == > WAVE_FORMAT_EXTENSIBLE) || fmt->nChannels > 2){ > WAVEFORMATEXTENSIBLE *fmtex = (void*)fmt; > DWORD mask, flag = SPEAKER_FRONT_LEFT; > UINT i = 0; > -- > 2.0.4 > >

2 2

Re: [PATCH 6/8] reg: Add wchar/raw data conversion functions
by Stefan Dösinger 07 Nov '14

07 Nov '14

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-11-06 22:17, schrieb Jonathan Vollebregt: > +static LSTATUS wchar_get_data( const WCHAR *input, const DWORD type, > + const WCHAR separator, DWORD *size_out, BYTE **out) This whitespace use is a bit weird. Is there a precedent for it? > + ** (DWORD **) out = i; The space between ** and ( is a bit odd too. > + /* If it's the first character or the previous one was a separator */ > + if (!p || temp[p - 1] == 0) > + { > + HeapFree(GetProcessHeap(), 0, temp); > + return ERROR_BAD_COMMAND; > + } What does advapi32 do if you have empty substrings? (I.e., the same question as with the trailing backslashes: Does this code belong into reg.exe or advapi32.dll?) If advapi32 returns an error you can probably use it instead of the internal ERROR_NOT_INT_OR_NEG. > + *out = HeapReAlloc(GetProcessHeap(), 0, temp, p * sizeof(WCHAR)); Technically this could fail. But I don't think it is necessary - the compiled code for the HeapReAlloc invocation takes probably more space than the few bytes you're freeing a little bit earlier. > + return ERROR_SUCCESS; > + } > + case REG_BINARY: > + { > + BYTE * temp = HeapAlloc(GetProcessHeap(), 0, strlenW(input)); Inconsistent style. > + case REG_BINARY: > ... > + return ERROR_BAD_COMMAND; Does native write different error messages for a negative DWORD, improperly separated REG_MULTI_SZ and invalid REG_BINARY? > + *out = HeapReAlloc(GetProcessHeap(), 0, temp-odd, p); MSDN says the third parameter has to be a pointer returned by HeapAlloc or HeapReAlloc. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJUXLpiAAoJEN0/YqbEcdMwEB4P/R4usEhK7b6woRAHZWXh3oSg UfhpgUpQQELq0nw2WMQrVtkhwvcJywSAkaI/MPyB0vDR6DScqTrcDe2j78yIyMiY DWJ24q+Xfix84fiOfSKbMWJZMAcZnCfn0pliwN/TjfzjOLdAZc6B0fO84wEJfHLL DstJzYgzANg1u3FZJtNpAgex2PpU8MpyATXqbvoZdPhJO3MCwuD9KVzqHgoGupbI A1BIfLWTE3ZHj+x8ygGOQcG4uhcsxWJQu+ygKvoDxGkjfQcQ1gEa9KOmWENDUj+Y 8eT+Ghzn+B7Ekl6RjgKGtNJKgVycwPqWJwR0JgWKRVlkeGrAfl6RNerAPwTODHj0 S+vmYeo6ZtrmHluagnP/hk2SB0yr6DmUITdhfE7LpMKAGutQ32PN1n5eDKpGHrez LWcvJ1beLCEVZk2fJe1r7pqZtMWPIu1wJmla+NyS/7UOuagHcskINm3cSsqNApRn ujMNALDGYozDhMl5/QJi9TJaqcwB2iRocDppkB75XnpcgLuKIION3KNi3jEhsBF8 DMIIF6/cPPFMSqvsoMcaFgm1CIz+AP5yo0LSfR/Rb2RGh+nhWFmBcNyLO6/bIWih D2ebrfCfq+SEzS/bqm3zoAXwShw3464xNuU/m9N8eF7erwUY8o2TDn+XwtXaoOlF kBkaNk8deBwwhIjLG6Pm =wFNB -----END PGP SIGNATURE-----

2 1

Re: [PATCH 8/8] reg: Clean up reg_delete
by Stefan Dösinger 07 Nov '14

07 Nov '14

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-11-06 22:17, schrieb Jonathan Vollebregt: > + FIXME("Trying to delete a root key is a bad > idea.\n"); I'd phrase this as FIXME("Deleting root keys is not implemented\n"); or similar. That makes it clear that there's something in our code that needs improvement. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJUXMIXAAoJEN0/YqbEcdMwId4P/iDoiGb5zkUHmHICCUGtgQRY h1xl20pabBDu6CVqGfvGxEVAPvWiRU3NHGv/ZmhH7TIv/1bjiFQjPjGlHoxOQmQY +FJspMCbaYfc63pCUh0Z0ogAUImef3yDf+DPYG4ybNgm5mqn8dR8hqpuUeyw2IIa NINo8JvZAd3markcKo8nOnGtrPukAGMbROOBvbkszWJSAiFU4jLMNyGpRcnwmuer ks2Y6sx2GyAAcuIGhIk4LUQegMzltIU+pEpEqffMNqbpqJ9nCd40PNcXCa2Wu9F6 o5ld2Pxfjp5kn/3t0V2zOVfkpRWZJw8+tba9eVUSN3MheV/DNKnbwdF46au8dLsQ shvfFyPugamA0zOKIbY3RJynlUSo8Gj8rLlT9rMX/qrF7WVIVQcFj0/pywmMUN3z fg7CnYDQSglHMVueXUmeHIZrvA9cXJHcDAs31wZbxs72EWnpa4lDnHOgMto5GydB 6cCw/2dw1DHE4BY/xsQNXFJKN+w5fRjkZXgXAEig7R0CTceGHPXjx4xAx/L7L83E 3z/HIv+K/oZ18wCg4y9R/X3fEnLyVR1d5EWcYQjpJ3KbvUloBUEbjeJi+pC9EBHN ilt2/KD3EUW9FAz072VlGcnYCQ1gx2h+II8q2SE3vwTQqyM458c708URJXnnnAyG RKG8p5SmURsRIQ1hiXjc =SPe6 -----END PGP SIGNATURE-----

1 0

Re: [PATCH 5/8] reg: Add wchar/type conversion functions
by Stefan Dösinger 07 Nov '14

07 Nov '14

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, Am 2014-11-06 22:17, schrieb Jonathan Vollebregt: > + {REG_NONE, type_none}, An idea for a future test: Test if REG_NONE does anything useful. That's not a blocker for this patch though. > +static DWORD wchar_get_type(const WCHAR *type_name) > { > ... > + for (i = 0; i < ARRAY_SIZE(type_rels); i++) > + { > + if (!strcmpiW(type_rels[i].name, type_name)) > + return type_rels[i].type; > + } > > return -1; > } > ... > if (reg_type == -1) Dmitry shout at me, but ~0u would be better since DWORD is unsigned. If REG_NONE doesn't do anything useful you could use REG_NONE for this purpose. That's just a suggestion though, ~0u is fine for me. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJUXLWtAAoJEN0/YqbEcdMwfVEP/Ap54MfEFnTiDeM9g7/ps9Gf 41SSLz0IJLgNf+UZlifdS1jajIL7lYM6jo91skiJXPfu36kdQeGvOEwJJhCmrPH3 lvdDMetjOKj6Zx7eoawYIi1ZhUSavp6ZMeLT0zBjdzHN14fHZHlccVnw/Jmb/Gd6 zq04scw0wRenWHobb69eW6dAQXy1N9bz9o9wavA6wvVKWo+k5EMSfJB+F3wGMJHN M8/eENJLBbZcpig2Rrlu8xUG1qVHjT01rjmwEJKAl83nj0cfrvWsudm6addZFL/H mLRjJfvZcLe9qBs8gUNtzgdDNqRCjuRjHHcV7qhn7TV/VsHLr68vuvLmLrbDX/vE G9x6jbA9C/Ob8L1QyMTeJyE2aCfVklHGuGpiS8NmzLNT974tllWTnXgA9tkeXQNC xt/hEoHXlTW9o+Dv70XLbdf6Y2Cw6rrDFGRDvUUAjTGPyRG6R8zTv1Jex4KkxJiH 8/cgm552PU2SIoqNneXbQswbJ7OOGwgi64J24YzfD8x2IpT4FaIqpXqZ0dGh00iO Y/XbA57yA8gi9hRrP2xC8WODHJBctRizhW9JCRKZp4jDb3/gKoK35WWSNY3jlku5 8/fTCv94gHv+K08GH4XdJ+Zm5nSjSLhjCkx0R3KwKuf5ansmPzmSysjEqfjQs8es RcGjE3c16jM532UVuHa8 =Cjz7 -----END PGP SIGNATURE-----

1 0

Re: [PATCH 2/8] reg: Add sane_path function to do preliniary key validation
by Stefan Dösinger 07 Nov '14

07 Nov '14

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-11-06 22:17, schrieb Jonathan Vollebregt: > diff --git a/programs/reg/tests/reg.c b/programs/reg/tests/reg.c > index f93df55..57e0108 100644 > --- a/programs/reg/tests/reg.c > +++ b/programs/reg/tests/reg.c > @@ -129,10 +129,10 @@ static void test_add(void) > ok(err == ERROR_FILE_NOT_FOUND, "got exit code %d\n", r); > > run_reg_exe("reg add HKCU\\" KEY_BASE "\\keytest2\\\\ /f", &r); > - todo_wine ok(r == REG_EXIT_FAILURE || broken(r == REG_EXIT_SUCCESS /* WinXP */), > + ok(r == REG_EXIT_FAILURE || broken(r == REG_EXIT_SUCCESS /* WinXP */), > "got exit code %u\n", r); > err = RegDeleteKeyA(HKEY_CURRENT_USER, KEY_BASE "\\keytest2"); > - todo_wine ok(err == ERROR_FILE_NOT_FOUND || broken(err == ERROR_SUCCESS /* WinXP */), > + ok(err == ERROR_FILE_NOT_FOUND || broken(err == ERROR_SUCCESS /* WinXP */), > "got exit code %d\n", r); > > run_reg_exe("reg add HKCU\\" KEY_BASE "\\keytest3\\ /f", &r); I think this part should be moved into the next patch. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJUXK9OAAoJEN0/YqbEcdMwonwP/j9INboZdEyYIP9ShVkn3W/U vK0kn4R+dzlMsoHrxyQI8037A8SnppqFj6QcvaPBmgj682srl0NdWKlPZPYPYOp5 DVmio9kN4w3KtZ9Oezs+mMXEUTHJ4sSE1fHOkn3TujqCLqKT3Ek6ANl8yFuTUWYz 27Usan0HPlW3oMGgyhTgdltE/v0Q3ObHXMvhoKsYI9lWSHEA1stjtk0CSUMe6mdG hE2ftJb2+nhVoFckNf9IcFMaIj6Ctizi7+oLU6bUNwl4zYN5sClyQto/YwPG6n1x zRARhrDYE+2g8N7dgkdBv8v98sCe8F8O6rMIz98BcE75sof3sNxCeiHqKqhFGkMP KWe6OHungsXxNy/tA/RzB6mKTL334KBwFrGz+qsdHB/ikj0mD8ykrY0aWnPmAw4K GqxpFFMl2AFe3p65cTzZP9TQ2QpiMZt5nkZbqdSABP4nqrUsYEhnact95KrBu8R0 Lcq9ahxgs1R76BRWnfu1VoNVPKyJK6iTiXoKmUbvS8CMhq/CW9rZs5GCKzso/oSY c1wt5Lgc5ZBFUaBMhK3fdSQbfw33iTpCqLbrERarrvD0SMUvvKrN4sBYE/4pFaEU JkGnNb02YB1SrLKOE7TkMZOc9npfk/aLPEfH7q0UCmYqN2wKidMaEk9rdINm5l0S i8KHvSnvIWXixdOcN00z =a//X -----END PGP SIGNATURE-----

1 0

Re: [PATCH 1/8] reg: Add system error printing function
by Stefan Dösinger 07 Nov '14

07 Nov '14

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-11-06 22:17, schrieb Jonathan Vollebregt: > +static int reg_print_error(LSTATUS error_code) As mentioned yesterday this doesn't need a return value. > + FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, NULL, > + error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (WCHAR *)&message, 0, NULL); > + > + reg_message(STRING_ERROR); > + reg_printfW(error_string, error_code, message); > + HeapFree(GetProcessHeap(), 0, message); http://msdn.microsoft.com/en-us/library/windows/desktop/ms679351(v=vs.85).a… says FormatMessage uses LocalAlloc to allocate the buffer. You need to free it with LocalFree. I don't know if we have any rule for checking the return value for FormatMessage here. You are already in an error printing codepath, and if you print an error every time printing an error fails you'll never stop. Otoh it seems questionable not to check the return value. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJUXK8aAAoJEN0/YqbEcdMwxuoP/RGyxXJoVN5h8Quj3L5HFGb+ Fg3xah51LzRJzQFirZ7yF13OVoZiGg7om4eAlo/eSAy1Ae/jzGxodg7PKEU7DXX/ 3s2TYE9TrkPJClt2hFq1asGoAI1uezNDJybwSXBxEgRAkUzBBhBh+9XEKffZz3iE OWpyHyCOQBwHPBq3krLP+joADngT13UfEvvrccNobvYb47WMBddJ70UdD+sYeF/i fbbYMdnhIKowbe26xCVwSDN5huXMq1f9zYuqkJwL6BRmkgD5FiwYdo8zXu2BvpEq ZkFwxUjCXFRJ7nwLAly5lNSCA1MMFD/Whwd5PxbMmVCwWK3nhqknxQdJJJIy95vd e2JeSwk/4ELavjXubMGAH5D7QfTnveO/ji/kZCWB2buN30ht5ORRH6bjupL/zIr4 hhh4HJ/6EdHhWJfffenmM9uGxN8zNO6HmVSHVnXn90QnSousjuNkFymfr9RkWycr kmjPJQw8inoEl0zmeMiDdu+Kxo9MHX7SAwAHi9OzYeU2aka8LTOASn0mmKM+GBeO 9uaJbQAa/ILzWkFaYY/C+lgnyJgjgY6+fFj1O+DFlXh4DnaZ1DXMrn4+3WrUDWQk 3cFD0/6ehVBuUouEY8dF0yDiWPO9cwTEe7S5G9u03rS72H6mRfRP8mJVI6ZVPwbU 7wj5TGWDXCcDiRHd46UI =abxe -----END PGP SIGNATURE-----

1 0

Re: [PATCH] vbscript: Implemented String(try 3)
by Piotr Caban 07 Nov '14

07 Nov '14

On 11/07/14 04:36, Shuai Meng wrote: > used mod other than division when the second argument(length of > string) is grater than 256; Your test was already showing that it doesn't work this way. You will probably need to use MultiByteToWideChar function here. + case VT_ARRAY|VT_BYREF|VT_VARIANT: + return DISP_E_TYPEMISMATCH; + default: + hres = to_short(arg + 1, &tmp); There's no need to handle VT_ARRAY|VT_BYREF|VT_VARIANT case separately. It will be done by to_short function. Thanks, Piotr

1 0

Re: shell32: Simplify check for empty string (PVS-Studio)
by Alistair Leslie-Hughes 07 Nov '14

07 Nov '14

On 07/11/14 20:22, Michael Stefaniuc wrote: > --- > dlls/shell32/shellord.c | 2 +- > dlls/shell32/tests/ebrowser.c | 2 +- > dlls/shell32/tests/shlfolder.c | 14 +++++++------- > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/dlls/shell32/shellord.c b/dlls/shell32/shellord.c > index 56c9fe9..914dc5a 100644 > --- a/dlls/shell32/shellord.c > +++ b/dlls/shell32/shellord.c > @@ -947,7 +947,7 @@ void WINAPI SHAddToRecentDocs (UINT uFlags,LPCVOID pv) > /* buffer size looks good */ > ptr += 12; /* get to string */ > len = bufused - (ptr-buffer); /* get length of buf remaining */ > - if ((lstrlenA(ptr) > 0) && (lstrlenA(ptr) <= len-1)) { > + if (ptr[0] && (lstrlenA(ptr) <= len-1)) { You are changing tabs to spaces here. Best Regards Alistair Leslie-Hughes

2 1

Re: [PATCH] vbscript: Implemented String(try 3)
by Marvin 07 Nov '14

07 Nov '14

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=10094 Your paranoid android. === w2000pro (32 bit) === No test summary line found === wxppro (32 bit) === No test summary line found === wvistau64 (32 bit) === No test summary line found === w2008s64 (32 bit) === No test summary line found === w7u (32 bit) === No test summary line found === w8 (32 bit) === No test summary line found === wvistau64 (64 bit) === No test summary line found === w2008s64 (64 bit) === No test summary line found

1 0

Re: [PATCH 2/4] d3d8/tests: The device restores the mode settings from the registry (v5).
by Marvin 07 Nov '14

07 Nov '14

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=10090 Your paranoid android. === w8 (32 bit device) === device.c:1173: Test failed: TestCooperativeLevel failed, hr 0x88760868. device.c:1218: Test failed: Reset failed, hr 0x88760868. device.c:1220: Test failed: TestCooperativeLevel failed, hr 0x88760869. device.c:1224: Test failed: Got unexpected value 0 for D3DRS_LIGHTING. device.c:1231: Test failed: D3DVIEWPORT->X = 10, expected 0. device.c:1232: Test failed: D3DVIEWPORT->Y = 20, expected 0. device.c:1233: Test failed: D3DVIEWPORT->Width = 400, expected 800. device.c:1234: Test failed: D3DVIEWPORT->Height = 300, expected 600. device.c:1235: Test failed: D3DVIEWPORT->MinZ = 2.00000003e-001, expected 0. device.c:1236: Test failed: D3DVIEWPORT->MaxZ = 3.00000012e-001, expected 1. device.c:1241: Test failed: Screen width is 640, expected 800. device.c:1242: Test failed: Screen height is 480, expected 600. device.c:1248: Test failed: Back buffer width is 640, expected 800. device.c:1250: Test failed: Back buffer height is 480, expected 600. device.c:1280: Test failed: Screen width is 640, expected 1024. device.c:1281: Test failed: Screen height is 480, expected 768. device.c:2441: Test failed: Expected foreground window 000B00F8, got 0002004C. device.c:2813: Test failed: Expected device window style 0x14cf0000, got 0x4cf0000. device.c:2817: Test failed: Expected device window extended style 0x108, got 0x100. device.c:2839: Test failed: Failed to reset device, hr 0x88760868. device.c:2843: Test failed: Expected device window style 0x14cf0000, got 0x4cf0000. device.c:2847: Test failed: Expected device window extended style 0x108, got 0x100. device.c:3215: Test failed: Failed to reset device. device.c:3222: Test failed: Expected {0, 0, 1024, 768}, got {0, 0, 512, 384}. device.c:3230: Test failed: Expected wndproc 0x412ed0, got 0x412ed0.

1 0