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

Re: [1/3] dpnet: Add check for mismatched string lengths (try 4)
by Marvin March 5, 2015

March 5, 2015

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=11922 Your paranoid android. === w8 (32 bit address) === The test timed out

1 0

Re: [PATCH 2/3] include/ole32/user32/winemac.drv/winex11.drv: do not allow dropping drags through obscuring native windows
by Ken Thomases March 4, 2015

March 4, 2015

Hi, On Mar 1, 2015, at 10:32 PM, Damjan Jovanovic <damjan.jov(a)gmail.com> wrote: > Currently Wine does not support dragging from a Wine window to a > native Window, but does from a Wine window to another Wine window. > What is a big surprise though, is that if a Wine window is obscured by > a native window that's on top of it in the z-order, it looks like you > are dropping onto the native window, but you are actually dropping > THROUGH the native window into the Wine window - in other words, the > wrong application gets the drop! > > This patch fixes that problem by giving ole32 awareness of native > windows (represented by a ULONG_PTR) via new Wine-specific functions > in user32 that are implemented by the display drivers winex11.drv and > winemac.drv (of which only the former is implemented by this patch). > The ole32 drag loop can thus tell whether the window under the mouse > is a Wine window or a native window, and obtain IDropTarget from > either window type, something that is used by the next patch to > implement dragging from Wine to native windows. I appreciate your work on this, but I think this isn't the right approach. I think if you solve the other problem (dragging from Wine to other apps) by having the driver take over drags and translating them into a native drag operation, then this problem gets solved automatically. The native drag operation will target the proper window under the cursor, eliminating the problem of dropping through non-Wine windows. If the drag goes over a Wine window, then the native drag will get translated back into Win32 by the existing code in the drivers for receiving drops. I understand your point from the next patch email that this may result in code duplication between the drivers, but I think that's largely inevitable because of the requirements of driving a native drag operation. Also, I don't believe your design where the driver implements IDropTarget and ole32 calls its methods can be made to work on the Mac. As is often the case, Cocoa has a much more closed API for drag and drop than X11. We can't really arbitrarily do pieces of the drag operation in isolation and effectively start and stop the drag (as the cursor moves from Wine window to native window and back to a Wine window). We can only start the drag and then Cocoa takes it from there. It calls relevant methods on the drag source object along the way, of course, but we don't have a lot of control over the drag operation other than that. -Ken

3 3

Re: [PATCH] vbscript: Implemented Asc(try 4)
by Marvin March 4, 2015

March 4, 2015

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=11905 Your paranoid android. === w2000pro (32 bit) === No test summary line found === wxppro (32 bit) === No test summary line found === w2003std (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] vbscript: Implemented Asc(try 3)
by Nikolay Sivov March 4, 2015

March 4, 2015

On 04.03.2015 9:19, Shuai Meng wrote: > + switch(V_VT(arg)) { > + case VT_NULL: > + return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE); > + case VT_BSTR: > + bstr = V_BSTR(arg); > + break; > + default: > + hres = to_string(arg, &bstr); > + if(FAILED(hres)) > + return hres; > + } > + > + ret = bstr[0]; > + SysFreeString(bstr); > + > + if(ret == 0) > + return MAKE_VBSERROR(VBSE_ILLEGAL_FUNC_CALL); > + return return_short(res, ret); This is wrong too. You can't always free it like that, please look carefully at where 'bstr' comes from.

2 1

Re: [1/3] dpnet: Add check for mismatched string lengths (try 3)
by Stefan Dösinger March 4, 2015

March 4, 2015

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, Am 2015-03-04 um 04:15 schrieb Alistair Leslie-Hughes: > + hr = IDirectPlay8Address_AddComponent(localaddr, DPNA_KEY_HOSTNAME, "testing", 8, DPNA_DATATYPE_STRING_ANSI); > + ok(hr == S_OK, "got 0x%08x\n", hr); This still uses a hardcoded size. I recommend to add an array const char test_str[] = "testing" similar to the localhost WCHAR. That way you can use the variable in all places. > + if (((strlenW((WCHAR*)lpvData)+1)*sizeof(WCHAR)) != dwDataSize) > + { > + WARN("Invalid STRING size, returning DPNERR_INVALIDPARAM\n"); > + return DPNERR_INVALIDPARAM; > + } There is a theoretical issue here that I don't know if we should care about. Assume e.g. dwDataSize is 10, but lpvData points to a blob that doesn't have a NULL byte before the end of the last defined page. In this case strlen will search for a string termination and segfault. strnlen could abort and return an error instead of segfault. I did some quick testing on Windows and it seems that this function uses a try / catch block to return gracefully in case of an invalid pointer. Generally we don't add tests that cause exceptions and I don't think we should test this here. And I have found no case where strnlen is used in Wine, and we have no strnlenW, so I don't think we should use it here. If I pass an invalid non-null pointer I get 0x80004003. If I pass a length = 0 I get DPNERR_INVALIDSTRING. Also checking !((char *)lpvData)[dwDataSize - 1] would be wrong, windows gracefully returns DPNERR_INVALIDPARAM on lpvData="123", size=999999. > + hr = IDirectPlay8Address_AddComponent(localaddr, DPNA_KEY_HOSTNAME, "testing", sizeof("Testing")+2, DPNA_DATATYPE_STRING_ANSI); > + ok(hr == DPNERR_INVALIDPARAM, "got 0x%08x\n", hr); Really minor issue: sizeof("testing") vs sizeof("Testing"). The result is the same, but a stupid compiler might place both strings in the executable. The variable I suggested above would avoid this issue here as well. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJU9zGdAAoJEN0/YqbEcdMw8MkP/0Mf46JYgZmA3IwqErfl5lRf fLq1ekHnsKWVXA8pMvl3A0bm2j9fP7UAErFo6mA+oW4KkqHtkqw9z87boJ5Ul5ko wy1T12fizUXXtxdq4Q+TKLCCl+SUX/CUS36Cvw7BeyHGldlUHUrgwQcv2UQg3T1Y dfug2KtrH1M8xTcZBmF9lRMGUqAUpcPxdH0wPh7AoOJsmNpXQUL3jZofsc2s6x7r JFffHxG2EdX96VSjhC+IG3vQLwH+SzlocblcGFFxhv/fjf0u0lgSZegmJVxDxfqc CVWSvMCpnXsd6O57ft8b25eJCmOkZLpYkvaOPiakj+Lz4I8h+lGzGck/Wfvy+bR2 Z0gqrNVjbABkYTlYHw6AsLHnliOnbvs3PROMG3mYsmmGOaLWIwFYhmIFBQCE6Kzv EWnvr3JSux2M689zurKvvJF/GYyS52SntMSpmJmcZ5TndFVi/xspL/+AUUMkr6tQ KK/ykTayv65ioa5T+VCFVclatama9wUcBkHQIU/ej9c4Njj9yXqTlBS7FC5Jieqo GBVHwuv0jNqMHBcAT5jR/V09wfbb3dmoz4yzdKJFGNRfpTeDFfF8qEbOO4AIN3Vy TRQQJJXXoz+UO+Ml0ZfYHxBsOnXSmiIHC8Pcdd9MTsY9XPUw1fFi+zLpXu8urm9Q vb7jaq/Ns9dXETrLE1m9 =jMxz -----END PGP SIGNATURE-----

1 1

Re: ntdll: Fix a spelling error.
by Nikolay Sivov March 4, 2015

March 4, 2015

> - * The returned unix_fd should be closed iff needs_close is non-zero. > + * The returned unix_fd should be closed if needs_close is non-zero. It's not necessary misspelled, 'iff' is a short form of 'if and only if'.

2 1

Re: msvcrt: Use Dragon4 algorithm to fix inconsistent rounding behaviour for sprintf and add tests. (try 2)
by Dmitry Timoshkov March 4, 2015

March 4, 2015

YongHao Hu <christopherwuy(a)gmail.com> wrote: > +typedef int int32_t; > +typedef unsigned char uint8_t; > +typedef unsigned long uint32_t; > +typedef unsigned long long uint64_t; Please use appropriate win32 types instead. Besides using 'typedef unsigned long uint32_t' is wrong. -- Dmitry.

2 1

Re: ws2_32/tests: Remove an unused assignment (PVS-Studio)
by Marvin March 4, 2015

March 4, 2015

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=11891 Your paranoid android. === w2003std (32 bit sock) === sock: unhandled exception c0000005 at 00000000

2 1

Re: [PATCH 1/4] wined3d: Properly up-scale WINED3DFMT_R5G5_SNORM_L6_UNORM.
by Matteo Bruni March 4, 2015

March 4, 2015

2015-03-02 22:29 GMT+01:00 Stefan Dösinger <stefan(a)codeweavers.com>: > Simply left-shifting is not enough in positive ranges. 0xff / 255 != > 0xff00 / 65535. > > Using the compiler to read 5 bit signed values seems a lot easier than > operating with masks. > --- > dlls/wined3d/utils.c | 44 +++++++++++++++++++++++++------------------- > 1 file changed, 25 insertions(+), 19 deletions(-) > > diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c > index 1ce23ac..67457d5 100644 > --- a/dlls/wined3d/utils.c > +++ b/dlls/wined3d/utils.c > @@ -290,6 +290,15 @@ static void convert_l4a4_unorm(const BYTE *src, BYTE *dst, UINT src_row_pitch, U > } > } > > +#include <pshpack1.h> > +struct r5g5l6 > +{ > + signed r : 5; > + signed g : 5; > + unsigned l : 6; > +}; > +#include <poppack.h> > + AFAIK C doesn't give any guarantee on a specific allocation of bitfields in memory. I'm sure this works in practice with GCC and maybe also with the other compilers we care about, but if it doesn't get too ugly I'd prefer explicit shifts and masks and be safe. In any case, this should use signed / unsigned short at least.

2 1

Re: [2/3] dpnet: Free previous allocated data (try 2)
by Stefan Dösinger March 4, 2015

March 4, 2015

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-03-04 um 04:15 schrieb Alistair Leslie-Hughes: > + if(entry->type == DPNA_DATATYPE_STRING_ANSI) > + heap_free(entry->data.ansi); > + else if(entry->type == DPNA_DATATYPE_STRING) > + heap_free(entry->data.string); > + else if(entry->type == DPNA_DATATYPE_BINARY) > + heap_free(entry->data.binary); This code is OK with me. What I think Nikolai meant was that you don't need the if / else if / else if checks and can just call heap_free(entry->data.binary) (or heap_free(entry->data.ansi) or heap_free(entry->data.string)) regardless of the type because it's just a union. However, there's a caveat, and this is why I think the code you have is reasonable: You don't want to free a non-pointer type (DPNA_DATATYPE_DWORD, DPNA_DATATYPE_GUID), so you need to check the type anyway. And if you have the code above or if (type == STRING || type == BINARY || type == ANSI) free(data.binary) doesn't really make a difference IMHO. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJU9um8AAoJEN0/YqbEcdMwh1wP/iQp3BbqRYT/5UqR+SJodAA+ 4MlKAmhdZ395k4G5AFAw6uZ1RnOxVjq0BH20jQbVo+l2bNiHPEsSQ0Ua9tcdQjR3 wA70dQpIQgL9/XCHxQZKfZN1ujEcAR0IsgOxqEW7mPQNwe/hRYPSLysubUJaYGeR cExeKFdabEM5pEIu+OwPxdvevRSVMA2Je38dlggMnIkq4ajJIM7I+x87GEJAxzaJ U3RsqqgTI2jvFspG5F12/O3hpynoAPUiHgsoQgzolMIcJLwqMaCFuM+Ify4kZotq h/Hdk8hR5BjBwAOJZZwJqyCYaulSVftRYH1k33N75o7LHOgwWDps/C7jzg7sgyLE F+187z1RBfWtvUmeJORRlezo1v9rceIqRX8H7rfKfsec500gF0KtVWcQxhENxfp/ +j5vgms5gSAtzvgig50Oq35GCULWspjzz4AnxzEB8BH/Z6ugE4zakejlAjUVMUVR T0iTKkhp25U7L4jl8TqKSJ82TZMh2Yc8ydrieO/XVNtMH5yjIw/A0Ecb+laKlwMy OfI9rQOUPyFhxu2dtxEn4MCgCussBF+Py9gL3mZ2ausfIttw8hH2nigb0sMud0YC oDPyTmVDK5ALvducoHi/QQhBoCh1duYkiscAj1n3IfYBNc/d89vKxmtEDr9O/jk1 QNm18QYpQzTXoprDPmQM =TILa -----END PGP SIGNATURE-----

1 0