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

Re: [PATCH 2/4] vbscript: Implemented Sgn(try 5)

by Piotr Caban

Hi, +Sub testSgnError(strings, error_num) + on error resume next + + Call Err.clear() + Call Sgn(strings) + Call ok(Err.number = error_num, "Err.number = " & Err.number) +End Sub + +Call testSgnError(Null, 445) This test fails on windows. Tests needs to pass both on windows and on wine. + if(V_VT(arg) == VT_NULL) { + FIXME("Null argument not supported\n"); + return E_NOTIMPL; + } Please fix the indentation here. Cheers, Piotr

July 2, 2014

Re: [1/2] ddraw: add dsurface dimension tests, try 11

by Henri Verbeet

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.

July 2, 2014

Re: [2/2] ddraw: add dsurface dimension tests, try 11

by Henri Verbeet

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.

July 2, 2014

Re: [1/2] imagehlp: Implement (parts of) BindImageEx (try 10)

by Jacek Caban

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

July 2, 2014

Re: [PATCH] riched20: Implement ITextSelection::GetText. (resend)

by Jactry Zeng

Hi Huw, 2014-07-02 18:08 GMT+08:00 Huw Davies <huw(a)codeweavers.com>: > > There's no need for an intermediate buffer here. Use > SysAllocStringLen() to allocate the BSTR and pass it to ME_GetTextW(). Thanks for your review! I will try again. -- Regards, Jactry Zeng

July 2, 2014

Re: [PATCH] riched20: Implement ITextSelection::GetText. (resend)

by Huw Davies

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.

July 2, 2014

Re: [PATCH 4/4] vbscript/tests: Added tests for the subtype of TypeName

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=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

July 1, 2014

Re: [PATCH 3/4] vbscript: Implemented Abs(try 5)

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=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

July 1, 2014

Re: [PATCH 2/4] vbscript: Implemented Sgn(try 5)

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=7802 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

July 1, 2014

Re: [PATCH 1/4] vbscript: Implemented VarType(try 5)

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=7801 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

July 1, 2014