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

Re: [PATCH] riched20: Implement ITextSelection::GetText. (try 2)
by Huw Davies 03 Jul '14

03 Jul '14

On Wed, Jul 02, 2014 at 08:01:32PM +0800, Jactry Zeng wrote: > --- a/dlls/riched20/richole.c > +++ b/dlls/riched20/richole.c > @@ -783,11 +783,32 @@ static HRESULT WINAPI ITextSelection_fnInvoke( > static HRESULT WINAPI ITextSelection_fnGetText(ITextSelection *me, BSTR *pbstr) > { > ITextSelectionImpl *This = impl_from_ITextSelection(me); > + ME_Cursor *start = NULL, *end = NULL; > + int nChars; > + > if (!This->reOle) > return CO_E_RELEASED; > + TRACE("%p\n", pbstr); > + if (!pbstr) > + return E_INVALIDARG; > > - FIXME("not implemented\n"); > - return E_NOTIMPL; > + ME_GetSelection(This->reOle->editor, &start, &end); > + nChars = ME_GetCursorOfs(end) - ME_GetCursorOfs(start); > + TRACE("%p, %p\n", start, end); This TRACE isn't very useful. Either print out the cursors' values or remove it. Other than that, this looks good to me. Huw.

1 0

Re: [PATCH (try2)] winecoreaudio.drv: Use AUHAL API instead of AudioQueue
by Ken Thomases 02 Jul '14

02 Jul '14

On Jul 1, 2014, at 2:53 PM, Andrew Eikum wrote: > try2: Resample outside of the callback thread. Actually try to use the > given format in IsFormatSupported. Well, the resampling has been moved off of the callback thread, but it happens with the spin lock held. So, it can still block the callback thread for the duration. Ideally, there wouldn't be a need for the spin lock; some sort of lockless data structure could be used to move buffers into and out of buffer lists. Given that a spin lock is used, we should try hard to keep the work done while it's held to a minimum. My first thought was that it should be possible to use a sort of double-buffering scheme. Within the lock, just swap some pointers so the callback thread can work with a different buffer. Then the resampling can be done outside of the lock. My second thought is that's exactly what the GetBuffer() / ReleaseBuffer() scheme is about. I think that Core Audio should map pretty cleanly to DirectSound. It should not be necessary to use a wrap buffer or a temp buffer. It should be possible to preallocate all buffers at the right size during Initialize (you can query the device, audio unit, and converter for the information necessary to compute the buffer size). We should be able to return the actual "device" period. It should be possible to avoid copying audio data. If we can't implement a lockless buffer list, then the spin lock would only be used to move buffers in and out of lists. Once the buffers are in the right list, the resampling can be done without the lock being held. All of that said, I seem to recall you tried to achieve something like that with the AudioQueue approach and kept hitting problems that prevented it. So, if I'm talking out of my ass, please disregard. One last thing: if no sample rate conversion is required (requested sample rate matches the device sample rate), it would be ideal to avoid using the converter. -Ken

1 1

Re: advapi32: Implement RegSetKeySecurity on top of NtSetSecurityObject.
by Alexandre Julliard 02 Jul '14

02 Jul '14

"Erich E. Hoover" <erich.e.hoover(a)gmail.com> writes: > @@ -2469,9 +2469,9 @@ LSTATUS WINAPI RegSetKeySecurity( HKEY hkey, SECURITY_INFORMATION SecurityInfo, > if (!pSecurityDesc) > return ERROR_INVALID_PARAMETER; > > - FIXME(":(%p,%d,%p): stub\n",hkey,SecurityInfo,pSecurityDesc); > + TRACE("(%p,%d,%p)\n",hkey,SecurityInfo,pSecurityDesc); > > - return ERROR_SUCCESS; > + return RtlNtStatusToDosError( NtSetSecurityObject( hkey, SecurityInfo, pSecurityDesc ) ); You'd need to handle special root keys, or at least have tests for them. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 3/4] vbscript: Implemented Abs(try 5)
by Piotr Caban 02 Jul '14

02 Jul '14

Hi, because of the test failures in previous patch you will need to resend this patch anyway. While doing it could you please remove additional newline character from: + assert(args_cnt == 1); + + + if(V_VT(arg) == VT_NULL) + return return_null(res); Thanks, Piotr

1 0

Re: [1/2] ddraw: add dsurface dimension tests, try 11
by Henri Verbeet 02 Jul '14

02 Jul '14

On 30 June 2014 17:42, Patrick Rudolph <siro(a)das-labor.org> wrote: > Subject: ddraw: add dsurface dimension checks > This subject line has some room for improvement. > + if ((desc->dwWidth > 0xffff) || (desc->dwHeight > 0xffff)) > + { > + HeapFree(GetProcessHeap(), 0, texture); > + return DDERR_INVALIDPARAMS; > + } Please just merge this with the other surface dimension check a couple of lines earlier.

1 0

Re: [2/2] ddraw: add dsurface dimension tests, try 11
by Henri Verbeet 02 Jul '14

02 Jul '14

On 30 June 2014 17:42, Patrick Rudolph <siro(a)das-labor.org> wrote: > + const static struct "static const" > + {16}, {0x0000F800}, {0x000007E0}, {0x0000001F}, {0x00000000} Please use lowercase hex constants. > + static struct > + { > + DWORD dwFlags; > + DWORD dwCaps; > + DWORD dwWidth; > + DWORD dwHeight; > + HRESULT hr; > + } Please drop the dw prefixes and capitals from the field names here. > + /* 6: Test maximum surface height */ > + {DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT, DDSCAPS_OFFSCREENPLAIN, 1, 0x10000, DDERR_INVALIDPARAMS}, > + /* 7: Test maximum surface width */ > + {DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT, DDSCAPS_OFFSCREENPLAIN, 0x10000, 1, DDERR_INVALIDPARAMS}, It would perhaps also be useful to verify that e.g. a 1 by 0xffff surface actually is allowed. > + /* 8: Test OUTOFVIDEOMEMORY */ > + {DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT, DDSCAPS_OFFSCREENPLAIN | DDSCAPS_VIDEOMEMORY, > + 0xffff, 0xffff, DDERR_OUTOFVIDEOMEMORY}, Double indentation here, please. > + ddcaps.dwSize = sizeof(DDCAPS); ddcaps.dwSize = sizeof(ddcaps); > + /* expect an error if the hardware doesn't support videomemory */ > + if (!(ddcaps.ddsCaps.dwCaps & DDSCAPS_VIDEOMEMORY)) > + tests[8].hr = DDERR_NODIRECTDRAWHW; You can just handle that when testing the expected result. (And then "tests" can just be const.) E.g.: hr = IDirectDraw_CreateSurface(...); if (tests[i].caps & DDSCAPS_VIDEOMEMORY && !(ddcaps.ddsCaps.dwCaps & DDSCAPS_VIDEOMEMORY)) ok(hr == DDERR_NODIRECTDRAWHW, ...); else ok(hr == tests[i].hr, ...); > + surfacedesc.dwFlags = tests[i].dwFlags | DDSD_PIXELFORMAT; It seems arbitrary to put DDSD_CAPS in the table, but not DDSD_PIXELFORMAT. IMO either both should be added here, or both should be in the table. > + hr = IDirectDraw_CreateSurface(ddraw, &surfacedesc, &surface, NULL); > + ok(hr == tests[i].hr, "Pixelformat %s, test %d returned %08x\n", formats[j].name, i, hr); > + > + if (surface) > + { > + IDirectDrawSurface_Release(surface); > + surface = NULL; > + } hr = IDirectDraw_CreateSurface(...); ... if (FAILED(hr)) continue; IDirectDrawSurface_Release(surface); > + ok(SUCCEEDED(hr), "CreateSurface returned %08x\n", hr); > + if (surface) The if is redundant here, since you're claiming that creating the surface should always succeed a line earlier. > + surface = NULL; This doesn't really do anything, since "surface" is never used afterwards.

1 0

Re: [1/2] imagehlp: Implement (parts of) BindImageEx (try 10)
by Jacek Caban 02 Jul '14

02 Jul '14

Hi Bernhard, On 06/30/14 19:15, Bernhard Reiter wrote: > diff --git a/dlls/imagehlp/tests/image.c b/dlls/imagehlp/tests/image.c > index e008b0b..d93130c 100644 > --- a/dlls/imagehlp/tests/image.c > +++ b/dlls/imagehlp/tests/image.c > @@ -31,6 +31,9 @@ > static HMODULE hImageHlp; > > static BOOL (WINAPI *pImageGetDigestStream)(HANDLE, DWORD, DIGEST_FUNCTION, DIGEST_HANDLE); > +static BOOL (WINAPI *pBindImageEx)(DWORD Flags, PCSTR ImageName, > + PCSTR DllPath, PCSTR SymbolPath, > + PIMAGEHLP_STATUS_ROUTINE StatusRoutine); > > /* minimal PE file image */ > #define VA_START 0x400000 > @@ -149,6 +152,9 @@ struct expected_update_accum > BOOL todo; > }; > > +static int status_routine_called[14] = {0}; > + > + > static BOOL WINAPI accumulating_stream_output(DIGEST_HANDLE handle, BYTE *pb, > DWORD cb) > { > @@ -273,6 +279,52 @@ static void update_checksum(void) > bin.nt_headers.OptionalHeader.CheckSum = sum; > } > > +BOOL WINAPI TestingStatusRoutine(IMAGEHLP_STATUS_REASON reason, PSTR ImageName, PSTR DllName, > + ULONG_PTR Va, ULONG_PTR Parameter) > +{ Make it static. > + char kernel32_path[MAX_PATH]; > + > + status_routine_called[reason]++; > + > + todo_wine ok((reason == BindImportModule) || (reason == BindImportProcedure) || > + (reason == BindForwarderNOT), "expected reason to be one of BindImportModule, " > + "BindImportProcedure, or BindForwarderNOT, got %d\n", reason); How about moving that to the switch that you need anyway? > + > + switch(reason) > + { > + case BindOutOfMemory: > + case BindRvaToVaFailed: > + case BindNoRoomInImage: > + case BindImportModuleFailed: > + case BindImportProcedureFailed: > + break; How about default: ok(0, "..."); > + > + case BindImportModule: > + ok(!strcmp(DllName, "KERNEL32.DLL"), "expected DllName to be KERNEL32.DLL, got %s\n", > + DllName); > + break; > + > + case BindImportProcedure: > + GetSystemDirectoryA(kernel32_path, MAX_PATH); > + strcat(kernel32_path, "\\KERNEL32.DLL"); > + ok(!lstrcmpiA(DllName, kernel32_path), "expected DllName to be %s, got %s\n", > + kernel32_path, DllName); > + ok(!strcmp((LPSTR)Parameter, "ExitProcess"), > + "expected Parameter to be ExitProcess, got %s\n", (LPSTR)Parameter); > + break; > + > + case BindForwarder: > + case BindForwarderNOT: > + case BindImageModified: > + case BindExpandFileHeaders: > + case BindImageComplete: > + case BindMismatchedSymbols: > + case BindSymbolsNotUpdated: > + break; > + } Same here. You really need cases for expected values. > + return TRUE; > +} > + > static void test_get_digest_stream(void) > { > BOOL ret; > @@ -329,6 +381,54 @@ static void test_get_digest_stream(void) > DeleteFileA(temp_file); > } > > +static void test_bind_image_ex(void) > +{ > + BOOL ret; > + HANDLE file; > + char temp_file[MAX_PATH]; > + DWORD count; > + > + /* Call with a non-existant file */ > + SetLastError(0xdeadbeef); > + ret = pBindImageEx(BIND_NO_BOUND_IMPORTS | BIND_NO_UPDATE | BIND_ALL_IMAGES, temp_file, NULL, > + NULL, (void *)TestingStatusRoutine); > + todo_wine ok(!ret && ((GetLastError() == ERROR_FILE_NOT_FOUND) || > + (GetLastError() == ERROR_INVALID_PARAMETER)), > + "expected ERROR_FILE_NOT_FOUND or ERROR_INVALID_PARAMETER, got %d\n", > + GetLastError()); > + > + file = create_temp_file(temp_file); > + if (file == INVALID_HANDLE_VALUE) > + { > + skip("couldn't create temp file\n"); > + return; > + } > + > + WriteFile(file, &bin, sizeof(bin), &count, NULL); > + FlushFileBuffers(file); > + CloseHandle(file); > + > + /* Finally, call with a proper PE file */ > + SetLastError(0xdeadbeef); > + ret = pBindImageEx(BIND_NO_BOUND_IMPORTS | BIND_NO_UPDATE | BIND_ALL_IMAGES, temp_file, NULL, > + NULL, (void *)TestingStatusRoutine); > + ok(ret, "BindImageEx failed: %d\n", GetLastError()); > + > + todo_wine ok(status_routine_called[BindImportModule] == 1, > + "Expected StatusRoutine to be called once with reason BindImportModule, " > + "but it was called %d times\n", status_routine_called[BindImportModule]); > + > + /* Under wvistau64 (64 bit image), w2008s64 (64 bit image) and w7pro64 (64 bit image), > + * StatusRoutine is called with reason BindForwarderNOT instead of BindImportProcedure. */ Please avoid such comments. Testbot VM names are not related to the code nor permanent. Cheers, Jacek

1 0

Re: [PATCH] riched20: Implement ITextSelection::GetText. (resend)
by Huw Davies 02 Jul '14

02 Jul '14

On Wed, Jul 02, 2014 at 05:09:47PM +0800, Jactry Zeng wrote: > + buffer = heap_alloc((nChars + 1) * sizeof(WCHAR)); > + if (!buffer) > + return E_OUTOFMEMORY; > + ME_GetTextW(This->reOle->editor, buffer, nChars, start, nChars, 0); > + buffer[nChars] = 0; > + /* FIXME: a '\r' should be appended at the end of a story */ > + *pbstr = SysAllocString(buffer); > + heap_free(buffer); There's no need for an intermediate buffer here. Use SysAllocStringLen() to allocate the BSTR and pass it to ME_GetTextW(). Huw.

2 1

Re: [PATCH 4/4] vbscript/tests: Added tests for the subtype of TypeName
by Marvin 01 Jul '14

01 Jul '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=7804 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 3/4] vbscript: Implemented Abs(try 5)
by Marvin 01 Jul '14

01 Jul '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=7803 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