December 2015 - Wine-devel - list.winehq.org

Re: [PATCH 1/2] dsound: Simplify error handling when creating a sound buffer.
by Sebastian Lackner 23 Dec '15

23 Dec '15

On 22.12.2015 10:49, Michael Stefaniuc wrote: > Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de> > --- > I run into this and wasted time while fixing the bug during the freeze. > So I am submitting the dsound cleanups I have laying around. > > > dlls/dsound/buffer.c | 46 ++++++++++++++++------------------------------ > 1 file changed, 16 insertions(+), 30 deletions(-) > Are you sure this will work as expected? The destructor makes assumptions about specific variables to be initialized. If I don't miss anything, DirectSoundDevice_RemoveBuffer could trigger an assertion / corrupt nrofbuffers now. This->buffer->ref will also not be initialized. Could you please double check that?

2 1

Re: ole32: Print error message when IPersistStorage_Load fails (Coverity)
by Huw Davies 23 Dec '15

23 Dec '15

On Tue, Dec 22, 2015 at 07:35:08PM +0100, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > CID 1336207 > > dlls/ole32/compobj.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c > index 2f55076..b146376 100644 > --- a/dlls/ole32/compobj.c > +++ b/dlls/ole32/compobj.c > @@ -3443,7 +3443,9 @@ HRESULT WINAPI CoGetInstanceFromIStorage( > > if (ps) > { > - IPersistStorage_Load(ps, storage); > + hr = IPersistStorage_Load(ps, storage); > + if (FAILED(hr)) > + ERR("failed to load storage\n"); > IPersistStorage_Release(ps); > } I suspect Coverity is expecting us to do something more useful with the return value ;-) How about returning the failure in this case (obviously having released ps first)? Also, just above this hunk, we should probably just return if QI fails. While you're at it, there are other CoCreateInstanceFrom* functions that could do with similar love. Huw.

2 1

Re: winspool: Remove dead code (Clang)
by Alexandre Julliard 23 Dec '15

23 Dec '15

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -4092,8 +4092,6 @@ static BOOL WINSPOOL_GetPrinter_1(HKEY hkeyPrinter, PRINTER_INFO_1W *pi1, > if(WINSPOOL_GetStringFromReg(hkeyPrinter, DescriptionW, ptr, left, &size)) { > if(space && size <= left) { > pi1->pComment = (LPWSTR)ptr; > - ptr += size; > - left -= size; > } else > space = FALSE; > *pcbNeeded += size; There's a clear pattern to the code here, you don't want to break it. I'd suggest redesigning the helper functions to avoid all this duplication. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] shlwapi: implement URL_ESCAPE_AS_UTF8
by Alexandre Julliard 23 Dec '15

23 Dec '15

Daniel Lehman <dlehman(a)esri.com> writes: > @@ -925,6 +955,20 @@ static void test_UrlEscapeW(void) > FreeWideString(urlW); > FreeWideString(expected_urlW); > } > + > + for(i=0; i<sizeof(TEST_ESCAPEW)/sizeof(TEST_ESCAPEW[0]); i++) { > + WCHAR ret_url[INTERNET_MAX_URL_LENGTH]; > + > + size = INTERNET_MAX_URL_LENGTH; > + ret = pUrlEscapeW(TEST_ESCAPEW[i].url, ret_url, &size, TEST_ESCAPEW[i].flags); > + if(ret == E_POINTER || TEST_ESCAPEW[i].broken) /* <= Win7/Win2k8 */ > + continue; That's not going to test anything for the broken cases. > @@ -1188,10 +1194,29 @@ HRESULT WINAPI UrlEscapeW( > if(cur == '\\' && (int_flags & WINE_URL_BASH_AS_SLASH) && !stop_escaping) cur = '/'; > > if(URL_NeedEscapeW(cur, dwFlags, int_flags) && stop_escaping == FALSE) { > - next[0] = '%'; > - next[1] = hexDigits[(cur >> 4) & 0xf]; > - next[2] = hexDigits[cur & 0xf]; > - len = 3; > + if(dwFlags & URL_ESCAPE_AS_UTF8) { > + char utf[4]; > + int i, ret; > + ret = wine_utf8_wcstombs(WC_ERR_INVALID_CHARS, &cur, 1, utf, sizeof(utf)); This won't work for Unicode surrogates. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [resend v2 2/4] ddraw7/tests: Test GetPriority() on cubemap surfaces.
by Marvin 22 Dec '15

22 Dec '15

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=19110 Your paranoid android. === w1064 (32 bit ddraw7) === ddraw7.c:2253: Test failed: Expected {0, 0, 512, 384}, got {0, 0, 1024, 768}.

1 0

Re: [resend 3/4] d3d9: Call wined3d_device_copy_sub_resource_region() instead of wined3d_device_update_surface() in d3d9_dev
by Marvin 22 Dec '15

22 Dec '15

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=19111 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [resend 3/4] ddraw: Check for mipmap sublevel through caps in ddraw_surface7_SetColorKey().
by Stefan Dösinger 22 Dec '15

22 Dec '15

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2015-12-21 um 18:38 schrieb Riccardo Bortolato: > - wined3d_mutex_lock(); > - if (!surface->wined3d_texture) > - { > - wined3d_mutex_unlock(); > + if (surface->surface_desc.ddsCaps.dwCaps2 & DDSCAPS2_MIPMAPSUBLEVEL) > return DDERR_NOTONMIPMAPSUBLEVEL; > - } > - wined3d_mutex_unlock(); This doesn't work here: ../../../../../src/wine/tools/runtest -q -P wine -T ../../.. -M ddraw.dll -p ddraw_test.exe.so ddraw7 && touch ddraw7.ok ddraw7.c:1767: Test succeeded inside todo block: Failed to set color key, hr 0. ddraw7.c:1772: Test succeeded inside todo block: Failed to get color key, hr 0. ddraw7.c:1773: Test succeeded inside todo block: Got unexpected value 0x000000ff. ddraw7.c:1775: Test succeeded inside todo block: Got unexpected value 0x000000ff. ddraw7.c:9485: Tests skipped: Overlays or some YUV formats not supported, skipping YUV colorfill tests. I guess you forgot to remove the todo_wine markers in my test. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWeb5HAAoJEN0/YqbEcdMwvQEP/0GfmyB5nebYlCy/Ffbt4lsE M5Nh6BXRnPlVbVwv1nLDEsujyfxWScAb6BHS6jx5QNnsCauv0/cpUDuzR8V4JHdf ltWqqgWBdqDJb0tG6JA89pw1sZwJxa2j92CE46GHSwozlEGX1LsufkeCJQ24XpqD rLD9TyHTTC8xncBr4BfryacbQgL+rp9OEuDLkvUqU1SyHf/OYyFuDRXhIg1rbflO 5PI+IxjFRN45sRKu4v073EcuPTM1BD8iFiAEAInhHcv+EBIHDqTmMcOl+r31w1Sb kWRcM7RMd7f2mLuWtT3UkHhRxo+nGeY3yhpQrYaKmaKJwzFGVPOfsVFPkPajIzLn iu++t1ZVX40m74WWTVJ6xPFFFaQ6CiPeFcUZquwtfaczTTba0lMwIRjpcQxdVC3R nkAZO/wWzVSWHJD4c8jB+I71uyFaE+Aj7AVLQJts8bFyyAwYizGvL2/Xn5Je1RJN PmX2HIobc0elWkL8qOpDzaJ7hRciLHXlWFB8Ce6GKHtJC90H72LgoKMaZBZFILNz EDwdvl7pMbSJJl2XqyiNmBhSVvBZMDzknLWE56qpprGkcGs6eyCSVOYSHswyDgK8 WjaFf81oQzfho+qH9as/tMRiPV/J2GZTs+IWt+47UX4bUVegTAn7wzOJRAh2tDg4 rV+rvEWoQZgm9g8lqKCq =Yx1L -----END PGP SIGNATURE-----

2 1

Re: [PATCH v2] loader: check return values of wld_mprotect and wld_mmap calls
by Sebastian Lackner 22 Dec '15

22 Dec '15

On 22.12.2015 12:22, Paul Gofman wrote: > Fixes loader crash when compiled using Intel compiler (icc 16.0.1 20151021). > > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > loader/preloader.c | 26 +++++++++++++++++--------- > 1 file changed, 17 insertions(+), 9 deletions(-) > Its not my decision, but I don't think its a good idea to add all the (unnecessary) ifs just to make this code compatible with ICC. I assume the compiler doesn't handle inline assembly correctly, but this would mean that there are several more affected places. It would be nice to have a more general solution to fix/workaround this issue. If this is not possible, I would at least suggest to add a workaround to the functions directly, instead of adding checks to all places where they are called.

3 8

Re: mmdevapi: Skip device if GetId fails (Coverity)
by Andrew Eikum 22 Dec '15

22 Dec '15

Here too, please print a WARN. Andrew On Tue, Dec 22, 2015 at 07:35:02PM +0100, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > CID 752979 > > dlls/mmdevapi/devenum.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c > index d50d206..5f0ffba 100644 > --- a/dlls/mmdevapi/devenum.c > +++ b/dlls/mmdevapi/devenum.c > @@ -1065,9 +1065,11 @@ static HRESULT WINAPI MMDevEnum_GetDevice(IMMDeviceEnumerator *iface, const WCHA > > for (i = 0; i < MMDevice_count; ++i) > { > + HRESULT hr; > WCHAR *str; > dev = &MMDevice_head[i]->IMMDevice_iface; > - IMMDevice_GetId(dev, &str); > + hr = IMMDevice_GetId(dev, &str); > + if (FAILED(hr)) continue; > > if (str && !lstrcmpW(str, name)) > { > -- > 1.9.1 > > > >

1 0

Re: winmm: Respect return value from joyGetPos (Coverity)
by Andrew Eikum 22 Dec '15

22 Dec '15

This section is already consistently using 8-space tabs, I'd rather not break that. Also I think you should print a WARN when joyGetPos returns an error. Andrew On Tue, Dec 22, 2015 at 07:34:59PM +0100, André Hentschel wrote: > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > CID 1261597 > > dlls/winmm/joystick.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/dlls/winmm/joystick.c b/dlls/winmm/joystick.c > index 6ac67f4..fb30ac8 100644 > --- a/dlls/winmm/joystick.c > +++ b/dlls/winmm/joystick.c > @@ -93,7 +93,8 @@ static void CALLBACK JOY_Timer(HWND hWnd, UINT wMsg, UINT_PTR wTimer, DWORD dwTi > > if (joy->hCapture != hWnd) continue; > > - joyGetPos(i, &ji); > + if (joyGetPos(i, &ji) != JOYERR_NOERROR) continue; > + > pos = MAKELONG(ji.wXpos, ji.wYpos); > > if (!joy->bChanged || > -- > 1.9.1 > > > >

1 0