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

Re: [PATCH 3/7] reg: Add path/key conversion functions

by Jonathan Vollebregt

On 11/06/2014 03:54 PM, Stefan Dösinger wrote: >> Reg is supposed to throw an error if the key ends in more than one >> backslash, RegCreateKey/RegOpenKey doesn't so it has to be checked >> manually. > Do we have tests for the RegCreateKey / RegOpenKey behavior? I did not > find any when I looked. I ran a quick one on testbot myself that confirmed it, but didn't write up a proper one. >> The message with ERROR_INVALID_DATATYPE says it indicates the >> *data* has the wrong type, nothing about the type itself. > Huh, I am not sure I understand what this means or what the difference > is :-\ . Huh. After looking up the error message I can't find it. Perhaps I was looking at a different error. Yeah ERROR_INVALID_DATATYPE looks good, I'll use that one. Speaking of - since the type_get_wchar and wchar_get_type functions return 2 states like path_get_rootkey, should I have them return pointers too? (Or DWORD -1 for error in the case of wchar_get_type) >> Given an incorrect type RegSetValueEx returns ERROR_ACCESS_DENIED, >> which isn't the most descriptive of error codes. Perhaps it would >> still be better to make our own more descriptive error code > Agreed, ERROR_ACCESS_DENIED is a bit unfortunate. This is the behavior > of Windows, not Wine's implementation, right? Yeah, both windows and wine return ERROR_ACCESS_DENIED

Nov. 6, 2014

Re: [PATCH 3/7] reg: Add path/key conversion functions

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-11-06 09:11, schrieb Jonathan Vollebregt: > On 11/04/2014 10:48 PM, Stefan Dösinger wrote: >> My reading comprehension may be lacking again, but can't you just >> call RegDeleteTree(subkey, NULL)? You're also assuming the caller >> isn't trying to nuke HKEY_LOCAL_MACHINE:-) . > > That deletes everything below the key, but not the key itself, so I > have to do it that way. This is unfortunate, I guess we'll have to live with parsing the key twice in this case. RegDeleteKey doesn't help either, it can only delete a subkey. > Reg is supposed to throw an error if the key ends in more than one > backslash, RegCreateKey/RegOpenKey doesn't so it has to be checked > manually. Do we have tests for the RegCreateKey / RegOpenKey behavior? I did not find any when I looked. > The message with ERROR_INVALID_DATATYPE says it indicates the > *data* has the wrong type, nothing about the type itself. Huh, I am not sure I understand what this means or what the difference is :-\ . > Given an incorrect type RegSetValueEx returns ERROR_ACCESS_DENIED, > which isn't the most descriptive of error codes. Perhaps it would > still be better to make our own more descriptive error code Agreed, ERROR_ACCESS_DENIED is a bit unfortunate. This is the behavior of Windows, not Wine's implementation, right? >> I think this is a bad place to catch unsupported data types. >> "type" comes from one of your own functions, namely >> wchar_get_type(). If reg.exe shouldn't support any of the data >> types not explicitly handled here then wchar_get_type should >> already return an error in this case. Otherwise you can make this >> a FIXME("Add support for type %u\n" type); > > What should I do in default then? It would be a bad idea to return > ERROR_SUCCESS For code that should be unreachable an ERR() would be enough. The completely unexpected already happened, any attempt to clean up after that will come up short anyway. The idea is that when ERRs are removed at compile time the compiler can remove the condition as well since it just covers an empty block of code. If you want to make sure you abort you can add an assert(0). Though if you put the conversion function into the type table you avoid the issue altogether. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJUW4uuAAoJEN0/YqbEcdMw4JQP/1h4Cg5/PlzgJCsKcEOzxlPc +A3M5St152rhcFQPmTM58QcwlGUBiZLMB1h/wmFYk+7fvADjLhv64BzFMvnhQ/B+ vIYDJBhcfAE0571W2H31bx76Rfl7BxviCWMTH+mUvI26okRBPbaQLC3TtwxaOaPO +UlRVq1F8y8q6fYqQ3qL/fHjml+swFf5n0vCyHMHFIDP+cBGzFvJmveW+HQowqTQ W/+c0thhj0+lekcZXQMNr4btfamhOcqU4D2YTEcFIRVg7+7QG0U89nri1C7S4DyW ZQh2c7T4P5VD+bv8KUgpowJmB91oZ3hv7RMyrLKdPKk1JKPs9UniA2T4BvPcEt1m i7k5QXOTIzYoaEccjaMEVnhOHExkhaiIZoJsMSOCoPF2jvzIDYL9f/4uCiLCKbnq 2lMilQ2k9PWxFI2JiuEeci8htAZVssyXtMZXUMjuWzoc2dkRAmhYOwWgSaSuZp+N chklFB+jwJLPtb3hcdzn4xAyx4ytECyknwoSt5b26iIx2Xlq6M4YYlVFzhpQ3P7M m/srFGITzSs26vo6loTAfBRrcRIAHVvvGxum5SLGvdf5ofPM6oeYfb6ejJ3bA06R JuShvacl0YdhTxwD+uziogVB3NbLPji0o2l3O7tCxHeI82NIihm09fOZi8G6f9PM BsrC/GG8E7Bn43Cv+j4A =4Il4 -----END PGP SIGNATURE-----

Nov. 6, 2014

Re: user32/tests: Add tests for MsgWaitForMultipleObjectsEx.

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=10062 Your paranoid android. === w7u (32 bit msg) === msg.c:12147: Test failed: unexpected hwnd 0014013C msg.c:12169: Test failed: bad time 308c6/308d6

Nov. 6, 2014

Re: [PATCH 1/5] d3d10core/tests: Port alpha_test() to d3d10core.

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=10061 Your paranoid android. === wvistau64 (32 bit device) === device.c:2175: Test failed: Failed to create input layout, hr 0x80070057. device.c:2210: Test failed: Failed to create texture, hr 0x80070057. device.c:2212: Test failed: Failed to create rendertarget view, hr 0x80070057. device.c:2260: Test failed: Got unexpected color 0x800000ff. device.c:2262: Test failed: Got unexpected color 0x800000ff. device: unhandled exception c0000005 at 7306CBBD === w2008s64 (32 bit device) === device.c:2175: Test failed: Failed to create input layout, hr 0x80070057. device.c:2210: Test failed: Failed to create texture, hr 0x80070057. device.c:2212: Test failed: Failed to create rendertarget view, hr 0x80070057. device.c:2260: Test failed: Got unexpected color 0x800000ff. device.c:2262: Test failed: Got unexpected color 0x800000ff. device: unhandled exception c0000005 at 744ACBBD === w8 (32 bit device) === device.c:2175: Test failed: Failed to create input layout, hr 0x80070057. device.c:2210: Test failed: Failed to create texture, hr 0x80070057. device.c:2212: Test failed: Failed to create rendertarget view, hr 0x80070057. device.c:2260: Test failed: Got unexpected color 0x800000ff. device.c:2262: Test failed: Got unexpected color 0x800000ff. device: unhandled exception c0000005 at 0040155D === wvistau64 (64 bit device) === device.c:2175: Test failed: Failed to create input layout, hr 0x80070057. device.c:2210: Test failed: Failed to create texture, hr 0x80070057. device.c:2212: Test failed: Failed to create rendertarget view, hr 0x80070057. device.c:2260: Test failed: Got unexpected color 0x800000ff. device.c:2262: Test failed: Got unexpected color 0x800000ff. device: unhandled exception c0000005 at 00000000745122BF === w2008s64 (64 bit device) === device.c:2175: Test failed: Failed to create input layout, hr 0x80070057. device.c:2210: Test failed: Failed to create texture, hr 0x80070057. device.c:2212: Test failed: Failed to create rendertarget view, hr 0x80070057. device.c:2260: Test failed: Got unexpected color 0x800000ff. device.c:2262: Test failed: Got unexpected color 0x800000ff. device: unhandled exception c0000005 at 00000000746C22BF

Nov. 6, 2014

Re: [PATCH 1/5] d3d10core/tests: Port alpha_test() to d3d10core.

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=10060 Your paranoid android. === w8 (32 bit visual) === visual.c:2635: Test failed: unexpected colortable on offscreen surface

Nov. 6, 2014

Re: [PATCH 3/7] reg: Add path/key conversion functions

by Jonathan Vollebregt

On 11/04/2014 10:48 PM, Stefan Dösinger wrote: > My reading comprehension may be lacking again, but can't you just call RegDeleteTree(subkey, NULL)? You're also assuming the caller isn't trying to nuke HKEY_LOCAL_MACHINE:-) . That deletes everything below the key, but not the key itself, so I have to do it that way. >> +static BOOL sane_path(const WCHAR *key) >> +{ >> + int i = strlenW(key); >> + >> + if (i < 3 || (key[i - 1] == '\\' && key[i - 2] == '\\')) >> + { >> + reg_print_error(ERROR_BADKEY); >> + return FALSE; >> + } > I think I have asked this a few times, please forgive me if I forgot the answer: Did you test if RegAddKey and friends handle this for you? > dlls/advapi32/tests/registry.c would profit from some tests for the error cases you have in programs/reg/tests/reg.c . Reg is supposed to throw an error if the key ends in more than one backslash, RegCreateKey/RegOpenKey doesn't so it has to be checked manually. >> +#define ERROR_INVALID_TYPE 20001 > What does RegSetValueEx return if you pass an invalid type? The API makes this error condition possible, so there's likely a return value for it. (Yeah, I know, it's not your fault advapi32/tests has no test for this. If I understand server/registry.c correctly such a call succeeds and creates a blob with an undefined type. Still needs tests) > > ERROR_INVALID_DATATYPE may be a candidate. The message with ERROR_INVALID_DATATYPE says it indicates the *data* has the wrong type, nothing about the type itself. Given an incorrect type RegSetValueEx returns ERROR_ACCESS_DENIED, which isn't the most descriptive of error codes. Perhaps it would still be better to make our own more descriptive error code > Another possibility is that native assumes a default value string of "" if /d is not specified, which then produces behavior like an empty REG_SZ and fails to convert into a DWORD. Nice catch! That fits perfectly! 29 loc less! >> + default: >> + return ERROR_UNSUPPORTED_TYPE; > I think this is a bad place to catch unsupported data types. "type" comes from one of your own functions, namely wchar_get_type(). If reg.exe shouldn't support any of the data types not explicitly handled here then wchar_get_type should already return an error in this case. Otherwise you can make this a FIXME("Add support for type %u\n" type); What should I do in default then? It would be a bad idea to return ERROR_SUCCESS

Nov. 6, 2014

Re: msvcp90: Add std_Ctraits::Isnan implementation.

by Nikolay Sivov

On 11/4/2014 11:22, YongHao Hu wrote: > --- > dlls/msvcp90/math.c | 9 +++++++++ > dlls/msvcp90/msvcp90.spec | 6 +++--- > dlls/msvcp90/tests/misc.c | 48 > +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 60 insertions(+), 3 deletions(-) > > > > You need to update msvcp80.spec too.

Nov. 6, 2014

Re: [PATCH 1/2] kernel32/tests: Add UTF-7 non-encoded surrounding characters tests.

by Sebastian Lackner

On 06.11.2014 06:17, Alexandre Julliard wrote: > Sebastian Lackner <sebastian(a)fds-team.de> writes: > >> Even if native would behave differently, and we probably would decide >> not to reproduce the behaviour - the tests are a good documentation of >> how it is supposed to work. The next person who wants to know if >> WideCharToMultibyte(CP_UTF7, ...) changes the error code, will have to >> write those tests just again. >> >> Any good reason behind this? > > With a few exceptions, Windows doesn't alter last error on success, so > its value depends more or less randomly on internal code paths, and on > whether there was a failure in one of the subfunctions. > > Since we don't replicate the internal code paths, the only way to pass > the tests would be to explicitly set last error on success. Of course, > as soon as we start doing that, we change the last error behavior of all > the callers of that function, which then need to set last error too, or > worse, save/restore it around subfunction calls. > > Pretty soon we'll be setting last error in every single code path, for > no benefit except passing a few misguided tests. > Thanks for the explanation. I have to agree that it definitely doesn't make sense to reproduce always the native error behaviour at all costs, but I personally don't see any big problem when tests check it anyway, especially when the behaviour is very consistent like in this example (error only modified when the function runs out of buffer and fails). What if these functions would for example change the error code when encountering invalid utf16 surrogates in the parsed data? I know that its not the case because its covered by the tests. Nevertheless, its your decision of course, and if this is the only issue which prevents you from accepting this patch, I am pretty sure Alex will be fine with changing that. ;) Regards, Sebastian

Nov. 6, 2014

Re: WineHQ Downloads page needs a disclaimer

by Scott Ritchie

Relatedly, if I can automate it, I'd be willing to provide a wine-vanilla package repo. On Wed, Nov 5, 2014 at 3:05 AM, Henri Verbeet <hverbeet(a)gmail.com> wrote: > On 4 November 2014 15:52, Rosanne DiMesio <dimesio(a)earthlink.net> wrote: > > A good example of why: https://bugs.winehq.org/show_bug.cgi?id=36964#c15 > > > > Our Downloads page doesn't directly say that the binary packages listed > are officially endorsed, but it does give that impression: only a few > distros are listed, the very top of the page says "Supported Wine" in big > letters, and while the 3rd Party Tools section explicitly says those aren't > supported, the Binary Downloads section gives no such warning. Users who > assume that the packages listed on that page have the "WineHQ seal of > approval" aren't being unreasonable. > > > > This isn't just an issue with the Fedora packges. The Ubuntu packages > with winepulse and changes to dsound have long been a problem (plus > https://bugs.winehq.org/show_bug.cgi?id=34299) and the CentOS/RHEL 7 > packages are 64 bit only. I have stickies on the forum about all these > things, but not everyone comes through the forum. > > > > The Downloads page needs to make it much clearer to users that the > distro packages listed are not "official" WineHQ packages. The wording > could be something like "Binary packages are built and maintained by the > distros, and may contain patches that are not supported here. The links > below are provided solely as a convenience to users and do not represent an > endorsement." > > > > (Posting here because that's what I've been told to do for such issues.) > > > I'd be inclined to agree, but the best way to actually make it happen > would be to send a patch against the website. > (git://source.winehq.org/git/website.git, > templates/en/download.template, etc.) > > >

Nov. 6, 2014

Re: [PATCH 1/2] kernel32/tests: Add UTF-7 non-encoded surrounding characters tests.

by Alexandre Julliard

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Even if native would behave differently, and we probably would decide > not to reproduce the behaviour - the tests are a good documentation of > how it is supposed to work. The next person who wants to know if > WideCharToMultibyte(CP_UTF7, ...) changes the error code, will have to > write those tests just again. > > Any good reason behind this? With a few exceptions, Windows doesn't alter last error on success, so its value depends more or less randomly on internal code paths, and on whether there was a failure in one of the subfunctions. Since we don't replicate the internal code paths, the only way to pass the tests would be to explicitly set last error on success. Of course, as soon as we start doing that, we change the last error behavior of all the callers of that function, which then need to set last error too, or worse, save/restore it around subfunction calls. Pretty soon we'll be setting last error in every single code path, for no benefit except passing a few misguided tests. -- Alexandre Julliard julliard(a)winehq.org

Nov. 6, 2014