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

Re: ntdll: Block signals while executing system APCs. (v2)
by Alexandre Julliard Nov. 4, 2015

Nov. 4, 2015

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Changes in v2: > * Do not delete the still valid comment "Return TRUE if a user APC has been run.". > I still had this deleted from an earlier version of the patchset, where the > return type was changed. > > If a USR1 suspend signal arrives between dequeing a system APC and sending > back the result to the wineserver, a deadlock occurs. To avoid this issue > this patch blocks all signals in server_select(), except while waiting or > processing user APCs. I'm guessing this is for bug 14697? It seems to me you could do that on the server side, by not suspending the thread while system APCs are outstanding. Also a test case demonstrating the bug would be a good first step. -- Alexandre Julliard julliard(a)winehq.org

3 7

Re: [PATCH 1/3] wined3d: Prepare the destination location in surface_load_location.
by Henri Verbeet Nov. 4, 2015

Nov. 4, 2015

On 3 November 2015 at 23:55, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > Instead of its sub-functions. See comments in patch 3 for details about > surface_blt_fbo. > > This fixes bug 39536. surface_load_location didn't prepare the > multisample renderbuffer. > > The next patches will consistently move the responsibility to the caller > of surface_load_location. This isn't necessarily wrong on its own, although I wonder if it's really worth it. I don't think it makes the API easier to use correctly, and I don't think there's going to be much of a performance impact. For fixing bug 39536 this is wrong though. If I'm reading the terminal output there correctly, the issue is that load_location() is called with WINED3D_LOCATION_RB_MULTISAMPLE, which it doesn't know how to handle. This patch will hide the issue by simply always calling prepare_location(). I'd have to check if multisample surfaces are supposed to have defined content after creation and presents, but perhaps what you need to do here is to add proper WINED3D_LOCATION_DISCARDED handling for multisample surfaces, similar to depth stencil surfaces.

2 2

Re: mshtml: Add IHTMLXMLHttpRequest::responseXML() method implementation.
by Nikolay Sivov Nov. 4, 2015

Nov. 4, 2015

On 04.11.2015 11:13, Zhenbo Li wrote: > + hres = IXMLDOMDocument_loadXML(xmldoc, str, &vbool); > + if(FAILED(hres)) { > + ERR("loadXML(%s) failed: %08x\n", debugstr_w(str), hres); > + IXMLDOMDocument_Release(xmldoc); > + SysFreeString(str); > + return hres; > + } This is not how it works. loadXML() returns S_FALSE on load failure as well, so you have to test strictly for S_OK, or check VARIANT_BOOL argument. You can probably add a test when invalid xml stream is received to see how _get_responseXML() behaves, e.g. does it return S_FALSE and a valid IDispatch or does it fail and return NULL. Also I don't think it's a good idea to trace xml contents.

3 3

Re: riched20: Set pStream->written in ME_StreamOutFlush.
by Huw Davies Nov. 4, 2015

Nov. 4, 2015

On Thu, Oct 29, 2015 at 01:20:57PM +0800, Jactry Zeng wrote: > diff --git a/dlls/riched20/writer.c b/dlls/riched20/writer.c > index 2c7386a..72a8db5 100644 > --- a/dlls/riched20/writer.c > +++ b/dlls/riched20/writer.c > @@ -63,6 +63,7 @@ ME_StreamOutFlush(ME_OutStream *pStream) > /* Don't resend partial chunks if nWritten < pStream->pos */ > } > pStream->pos = 0; > + pStream->written += nWritten; > return TRUE; > } While this is probably correct, it would be easy to add tests for this. We already have EM_STREAMOUT tests, so just add checks on their return values. Huw.

2 2

Re: dinput: Ensure X11 input events are forwarded to wineserver even without message loop.
by Marvin Nov. 4, 2015

Nov. 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=17964 Your paranoid android. === wvistau64 (32 bit mouse) === mouse.c:124: Test failed: IDirectInputDevice_Acquire() failed: 80070005 mouse.c:126: Test failed: IDirectInputDevice_Acquire() should have failed: 80070005 mouse.c:144: Test failed: Acquire() failed: 80070005

1 0

Re: [PATCH] odbccp32: Implement SQLGetPrivateProfileStringW/SQLGetPrivateProfileString (resend)
by Nikolay Sivov Nov. 4, 2015

Nov. 4, 2015

On 26.10.2015 11:56, Alistair Leslie-Hughes wrote: > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/odbccp32/odbccp32.c | 182 ++++++++++++++++++++++++++++++++---- > dlls/odbccp32/tests/misc.c | 223 ++++++++++++++++++++++++++++++++++++++++++++- > include/odbcinst.h | 4 +- > 3 files changed, 389 insertions(+), 20 deletions(-) This looks mostly fine. Let me suggest some cleanups. > > -int WINAPI SQLGetPrivateProfileStringW(LPCWSTR lpszSection, LPCWSTR lpszEntry, > - LPCWSTR lpszDefault, LPCWSTR RetBuffer, int cbRetBuffer, > - LPCWSTR lpszFilename) > +static HKEY get_privateprofile_sectionkey(LPCWSTR section, LPCWSTR filename) > { > + static const WCHAR odbcW[] = {'S','o','f','t','w','a','r','e','\\','O','D','B','C','\\',0}; > + HKEY hkey, hkeyfilename, hkeysection; > + LONG ret; > + > + if (RegOpenKeyW(HKEY_CURRENT_USER, odbcW, &hkey)) > + return NULL; > + > + ret = RegOpenKeyW(hkey, filename, &hkeyfilename); > + RegCloseKey(hkey); > + if (ret) > + return NULL; > + > + ret = RegOpenKeyW(hkeyfilename, section, &hkeysection); > + RegCloseKey(hkeyfilename); > + if (ret) > + return NULL; These 2 lines are redundant, you can remove them, or turn return line to always return hkeysection. > + > + return ret ? NULL : hkeysection; > +} > + > + if (!buff_len) > + return 0; > + > + if(!defvalue) > + buff[0] = 0; Please use consistent formatting, 4 leading spaces, and space after 'if'. > + > + if (!section || !defvalue || !buff) > + return 0; > + > + if (buff) > + buff[0] = 0; > + > + sectionkey = get_privateprofile_sectionkey(section, filename); > + if (sectionkey) > + { > + DWORD type, size; > + > + if(entry) > + { > + size = buff_len * sizeof(*buff); > + if (!RegGetValueW(sectionkey, NULL, entry, RRF_RT_REG_SZ, &type, buff, &size)) > + { > + usedefault = FALSE; > + ret = (size / sizeof(*buff)) - 1; > + } > + } > + else > + { > + WCHAR name[MAX_PATH]; > + DWORD index = 0; > + DWORD namelen; > + > + usedefault = FALSE; > + > + namelen = sizeof(name); > + while (RegEnumValueW(sectionkey, index, name, &namelen, NULL, NULL, NULL, NULL ) == ERROR_SUCCESS) > + { > + if ((ret + namelen+1) > buff_len) > + break; > + > + lstrcpyW(buff+ret, name); > + ret += namelen+1; > + namelen = sizeof(name); > + index++; > + } > + } What I don't understand is why temporary fixed size buffer is needed. This loop immediately copies to return buffer, it should be possible to enumerate directly to this buffer, and if return buffer is not large enough to hold all values it should be identical to this break from the loop you have. Is there a test for test by the way? For consistency it's better to turn RegEnumValueW to !RegEnumValueW, like RegGetValueW does, or the other way around, it depends on which style we use for the rest of dll already. Also not space at the end of parameter list. All of that applies to A-function too. > + > + ret = SQLGetPrivateProfileString("wineodbc", "testing" , "defaultX", buffer, 256, "ODBC.INI"); > + ok(ret == 8, "SQLGetPrivateProfileString returned %d\n", ret); > + ok(!strcmp(buffer, "defaultX"), "incorrect string '%s'\n", buffer); > + > + ret = SQLGetPrivateProfileString("wineodbc", "testing" , "defaultX", buffer, 4, "ODBC.INI"); > + ok(ret == 3, "SQLGetPrivateProfileString returned %d\n", ret); > + ok(!strcmp(buffer, "def"), "incorrect string '%s'\n", buffer); > + > + ret = SQLGetPrivateProfileString("wineodbc", "testing" , "defaultX", buffer, 8, "ODBC.INI"); > + ok(ret == 7, "SQLGetPrivateProfileString returned %d\n", ret); > + ok(!strcmp(buffer, "default"), "incorrect string '%s'\n", buffer); Please replace all magic numbers with strlen/strlenW/sizeof. > + > + strcpy(buffer, "wine"); > + ret = SQLGetPrivateProfileString("wineodbc", NULL , "", buffer, 256, "abcd.ini"); > + ok(ret == 14, "SQLGetPrivateProfileString returned %d\n", ret); > + if(ret >= 14) > + { > + ok(!strcmp(buffer, "testing"), "incorrect string '%s'\n", buffer); > + ok(!strcmp(buffer+8, "value"), "incorrect string '%s'\n", buffer+8); > + } Why testing this separately? It looks easier to test whole buffer at once, with memcmp if some embedding nulls are expected. With all that fixed, I think it's good to go.

1 0

Re: riched20: Implement superscript/subscript shortcut key.
by Huw Davies Nov. 3, 2015

Nov. 3, 2015

On Thu, Oct 29, 2015 at 02:02:23PM +0800, Jactry Zeng wrote: > diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c > index 98c7414..a54c0c7 100644 > --- a/dlls/riched20/editor.c > +++ b/dlls/riched20/editor.c > @@ -2442,6 +2442,32 @@ ME_KeyDown(ME_TextEditor *editor, WORD nKey) > return TRUE; > } > break; > + case VK_OEM_PLUS: > + if (ctrl_is_down) > + { > + CHARFORMAT2W fmt; > + > + ZeroMemory(&fmt, sizeof(fmt)); > + fmt.cbSize = sizeof(CHARFORMAT2W); > + SendMessageW(editor->hWnd, EM_GETCHARFORMAT, SCF_SELECTION, (LPARAM)&fmt); > + if (shift_is_down) > + { > + if (fmt.dwEffects & CFE_SUBSCRIPT) > + fmt.dwEffects &= ~CFE_SUBSCRIPT; > + fmt.dwMask = CFM_SUPERSCRIPT; > + fmt.dwEffects ^= CFE_SUPERSCRIPT; > + } > + else > + { > + if (fmt.dwEffects & CFE_SUPERSCRIPT) > + fmt.dwEffects &= ~CFE_SUPERSCRIPT; > + fmt.dwMask = CFM_SUBSCRIPT; > + fmt.dwEffects ^= CFE_SUBSCRIPT; > + } > + SendMessageW(editor->hWnd, EM_SETCHARFORMAT, SCF_SELECTION, (LPARAM)&fmt); > + return TRUE; > + } > + break; > case VK_ESCAPE: > if (editor->bDialogMode && editor->hwndParent) > PostMessageW(editor->hwndParent, WM_CLOSE, 0, 0); > Again, it shouldn't be too hard to add a test for this. We do similar tests by sending WM_KEYDOWN/UP messages to the control. Huw.

2 1

Re: [PATCH v3 2/7] msvcrt: Refactor pf_printf to take options as flags instead of as BOOL parameter
by Piotr Caban Nov. 3, 2015

Nov. 3, 2015

On 11/03/15 11:12, Martin Storsjo wrote: > From: Piotr Caban <piotr(a)codeweavers.com> I've screwed up sending improved version of the patch without correct "from" header. You're the author of the patch. Please resend it with correct author and without sign-off by me - I'll reply with a sign-off message. Thanks, Piotr

1 0

Re: [PATCH v3 6/7] ucrtbase: Handle the C99 'z' size_t specifier for integers
by Piotr Caban Nov. 3, 2015

Nov. 3, 2015

On 11/03/15 11:12, Martin Storsjo wrote: > + vsprintf_wrapper(UCRTBASE_PRINTF_LEGACY_MSVCRT_COMPATIBILITY, buf, sizeof(buf), "%zx %d", (size_t) 0x12345678123456, 1); Please avoid adding such long lines without a good reason. Thanks, Piotr

1 0

Re: [PATCH] configure: Fix compilation for systems without libpulse
by Bruno Jesus Nov. 3, 2015

Nov. 3, 2015

On Tue, Nov 3, 2015 at 9:00 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > Sorry about that, Francois. I think this will fix it. I took your > suggestion to use enable_winepulse_drv instead of PULSE_LIBS in the > later section. Hi, Andrew. I applied the patch and did a new configure && make, configure shows that I don't have Pulseaudio installed [1] but still I get the same problem when it hits winepulse.drv. Then I manually erased the Makefile inside winepulse.drv and did a second configure and make. Make recreated file still have -lpulse. [1] configure: libpulse 32-bit development files not found or too old, Pulse won't be supported. Am I missing something? I really don't understand this make stuff, sorry =/

3 3