November 2011 - Wine-devel - list.winehq.org

Re: [PATCH 3/7] ntoskrnl: intialze unknown fields of the irp with 0xce in IoInitializeIrp
by Alexandre Julliard 08 Nov '11

08 Nov '11

Bernhard Loos <bernhardloos(a)googlemail.com> writes: > @@ -864,6 +873,10 @@ VOID WINAPI IoCompleteRequest( IRP *irp, UCHAR priority_boost ) > > iosb = irp->UserIosb; > status = irp->IoStatus.u.Status; > + > + assert( status != STATUS_PENDING ); > + assert( status != 0xdeafbeef ); > + You can't assert on things that are set by the caller. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: kernel32/tests: add tests for GetDllDirectory (try 2)
by Alexandre Julliard 08 Nov '11

08 Nov '11

Thomas Faber <thfabba(a)gmx.de> writes: > + /* no buffer, determine length */ > + SetLastError(0xdeadbeef); > + ret = pGetDllDirectoryA(0, NULL); > + error = GetLastError(); > + ok(ret == length + 1, "Expected %u, got %u\n", length + 1, ret); > + ok(error == 0xdeadbeef, "Error is %u\n", error); Testing last error on success is generally not useful. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [10/11] ddraw/tests: Add assorted D3D3 DrawPrimitive/visual tests.
by Stefan Dösinger 08 Nov '11

08 Nov '11

Hi, Design-wise I think the 8 triangles and readback grid make the test fairly difficult to read. You could set up a structure that contains the d3d parameters you're changing between the tests and the expected color and then loop over the table. Something like this: const struct { BOOL d3drs_lighting; DWORD dp_flags; <- for things like D3DP_DONOTLIGHT DWORD fvf; void *vertices; DWORD expected_color; } test_data[] = { ... }; You can also think about sharing this control table between d3d2, d3d3 and d3d7 by having different color fields for that, but I don't know how well this will work. On Monday 07 November 2011 23:27:13 Octavian Voicu wrote: > + DWORD expected[] = { GRAY, WHITE, YELLOW2, YELLOW, GREEN, GREEN, BLUE, BLUE }; Please make data const where you can. > + /* Note: IDirect3DDevice3::DrawPrimitive calls with D3DVT_ vertex > types should fail. */ + > + /* Triangle 0 -- D3DFVF_VERTEX (vertices with normals, but without > colors). */ + hr = IDirect3DDevice3_DrawPrimitive(Direct3DDevice3, > D3DPT_TRIANGLELIST, D3DVT_VERTEX, > + vertices, 3, D3DDP_WAIT); > + ok(hr != DDERR_INVALIDPARAMS, "DrawPrimitive returned: %08x\n", > hr); I don't think there's a point in testing this, both D3DVT_* and D3DFVF_* are defines / enums that map to integers, and it is obvious that you get wrong behavior if you mix those. The ok condition looks strange. If you're trying to avoid a todo_wine, don't do that, just mark tests that show a difference between Wine and Windows as todo_wine. (I guess Wine currently returns DD_OK, and Windows returns D3DERR_INVALIDVERTEXFORMAT) > + /* Make sure getPixelColor works correctly. */ > + color = getPixelColor(device, 320, 240); > + ok(color == RED, "getPixelColor returned: %08x\n", color); We have some sanity checks for that in the main function Most of this applies to the d3d2 and d3d3 tests as well. I haven't looked over the cleanup patches yet.

2 1

Re: winscard/tests: Add the tests file (try 3)
by Bruno Jesus 07 Nov '11

07 Nov '11

On Mon, Oct 31, 2011 at 23:27, Bruno Jesus <00cpxxx(a)gmail.com> wrote: > try 3 - avoid magic constants by using winscard.h defines > try 2 - split the patches and fix win2000 error > > This is the starting point for the winscard implementation. I'll add > dozens of tests and I would like all of them to pass in windows before > we start using pcsclite. > Testbot will probably be useless since it does not have a smartcard > reader connected so I'll ensure all tests succeed in my local machine > before submitting them to wine (i'll append the test results on the patch email) > > In the next days I'll be updating the test file constantly by adding a > new tested function per patch. > Hi, I would like to know if there is anything wrong with this patch? It is currently working on testbot and introduces the tests file for the winscard dll. http://source.winehq.org/patches/data/80485 I really would like to get smart cards working until the wine l337 release. Thanks in advance, Bruno

1 0

Re: [PATCH 3/6] [UnInstall]: No longer use unix stdio functions, but the kernel32 equivalent
by Alexandre Julliard 07 Nov '11

07 Nov '11

Eric Pouech <eric.pouech(a)orange.fr> writes: > #28185 > > A+ > --- > > programs/uninstaller/main.c | 65 ++++++++++++++++++++++++++++++------------- > 1 files changed, 46 insertions(+), 19 deletions(-) I'm not sure that's a good idea. The output of uninstaller is meant to be used by Unix apps, it's not something a user would want to display on the console. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: winealsa: Don't set ALSA's period time.
by Alexandre Julliard 07 Nov '11

07 Nov '11

Joerg-Cyril.Hoehle(a)t-systems.com writes: > Hi, > > perhaps I shouldn't send this one day before release... > > I argue that Wine has no business setting ALSA's period size, > esp. not to a completely random value like duration/10. It doesn't work here: ../../../tools/runtest -q -P wine -M dsound.dll -T ../../.. -p dsound_test.exe.so capture.c && touch capture.ok capture.c:618: Test failed: Should not fail for 8 or 16 bits per sample capture.c:618: Test failed: Should not fail for 8 or 16 bits per sample make: *** [capture.ok] Error 2 -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: cmd: Add helper for 'if' comparison operations evaluation (try 2)
by Octavian Voicu 07 Nov '11

07 Nov '11

2011/11/7 Frédéric Delanoy <frederic.delanoy(a)gmail.com>: > + /* Note: '==' can't be returned by WCMD_parameter since '=' is a separator */ > + p = paramEnd + 1; > + while (*p == ' ' || *p == '\t') > + p++; > + > + if (!p || *p != '=' || !(p+1) || *(p+1) != '=') > + goto syntax_err; > + > + strcpyW(operator, eqeqW); > + p += strlenW(operator); Hi, This doesn't look general enough, I'm not sure how you want to handle the other operators. Wouldn't it be better to just find next space/tab (skip all non-whitespace chars) then assign the part of the string separated by whitespace to be the operator? With this approach you also don't need that syntax check there. Octavian

2 1

Re: [2/2] mscoree: Implement ICorDebug CreateProcess
by Alexandre Julliard 07 Nov '11

07 Nov '11

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> writes: > + { > + HANDLE handle, thread; > + DuplicateHandle(GetCurrentProcess(), lpProcessInformation->hProcess, > + GetCurrentProcess(), &handle, 0, FALSE, DUPLICATE_SAME_ACCESS); > + DuplicateHandle(GetCurrentProcess(), lpProcessInformation->hThread, > + GetCurrentProcess(), &thread, 0, FALSE, DUPLICATE_SAME_ACCESS); > + hr = CorDebugProcess_Create(This, (IUnknown**)&pDebugProcess, > + lpProcessInformation->dwProcessId, handle, thread); It would be better to duplicate the handles in CorDebugProcess_Create, and of course check for error and return an appropriate failure. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [1/2] ntdll: Setting WRITECOPY protection on an image section is allowed.
by Alexandre Julliard 07 Nov '11

07 Nov '11

Dmitry Timoshkov <dmitry(a)baikal.ru> writes: > This patch should fix the regression reported in the bug 28994. This needs more test cases, in particular for normal file mappings, as well as for VirtualAlloc behavior. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [1/2] kernel32/tests: add tests for GetDllDirectoryA
by Vitaliy Margolen 06 Nov '11

06 Nov '11

On 11/06/2011 09:14 AM, Thomas Faber wrote: > > + length = strlen(dll_directories[i]); > + assert(sizeof(buffer) > length + 1); Please don't do this. Tests shouldn't assert because programmer failed to pick the right buffer size of hard-coded strings. > + ok(GetLastError() == 0xdeadbeef, "Error is %u\n", GetLastError()); You should get value of GetLastError() before the ok check here. See all places that check values of GetLastError(). > + ok(buffer[length + 1] == 'A', "i=%d, Buffer overflow\n", i); > + ok(buffer[ret] == 0, "i=%d, Wrong length or not null terminated\n", i); > + ok(strcmp(buffer, dll_directories[i]) == 0, "i=%d, Wrong path returned: '%s'\n", i, buffer); You can do all these with one memcmp call. > > + ok(buffer[length + 1] == 'A', "i=%d, Buffer overflow\n", i); > + ok(buffer[ret] == 0, "i=%d, Wrong length or not null terminated\n", i); > + for (j = 0; buffer[j]; j++) > + if (buffer[j] != dll_directories[i][j]) > + { > + ok(0, "i=%d, Wrong path returned: %s\n", i, wine_dbgstr_w(buffer)); > + break; > + } Same for this. > + ok(buffer[0] == 0 || /* XP, 2003 */ > + buffer[0] == 'A', "i=%d, Buffer overflow\n", i); Please mark bad case with broke() to check for correct Wine behavior. Also please combine A & W tests. They do mostly the same things. Vitaliy.

2 1