re: [PATCH 2/2] cmd: Handle truncation for console reads
What's motivating this change? Are you quite sure you know how ReadConsole behaves? I bet one could write a test for this that used SendInput to send keystrokes to be read by ReadConsole, and verify the end of line and end of buffer behavior...
re: [PATCH 1/2] cmd: Improve WCMD_fgets documentation
+ * We can't use the native f* functions because of the filename syntax differences + * between DOS and Unix. That doesn't belong in a function comment; it's an implementation detail that belongs, at most, inside the function.
Re: Favorite bug: deadlock preventing backtraces?
On Fri, Sep 30, 2011 at 1:29 AM, Maarten Lankhorst wrote: > On 09/29/2011 06:00 AM, Dan Kegel wrote: >> On one of my boxes, if I run winmm/midi.ok in a loop, it will >> eventually deadlock/crash >> ( http://bugs.winehq.org/show_bug.cgi?id=28388 ): > > I'm wondering if it's because of the lock in virtual_handle_stack_fault, what > if you remove that? :P I did write a nice script to do a btall automatically when the lockup happens. With git, on my e8400, with WINEDEBUG=warn+heap, I get roughly 4 lockups per 100 runs. Under the same conditions, but without the lock in virtual_handle_stack_fault, I get ... roughly 4 lockups per 100 runs. And the stacks look the same.
Re: Using ApiTrace to improve D3D/OpenGL performance
I guess if you can reproduce the same exact frames, then probably you can get real FPS your videocard might crunch, duplicated operation and maybe improve/exclude paths of code which slow down the 'real' rendering? Cheers On 30/09/11 21:15, Henri Verbeet wrote: On 30 September 2011 22:07, Emanuele Oriani wrote: This might be useful to understand where some bottlenecks of wined3d might be? What do you think? Yeah, I'm aware of apitrace, through Mesa. I haven't used it for debugging wined3d so far, but I probably will at some point. I'm not sure it's really suited for profiling wined3d much though, other than for creating something reproducible to profile. (That's useful, but doesn't solve the entire problem.)
Re: Using ApiTrace to improve D3D/OpenGL performance
On 30 September 2011 22:07, Emanuele Oriani wrote: > This might be useful to understand where some bottlenecks of wined3d might > be? > What do you think? > Yeah, I'm aware of apitrace, through Mesa. I haven't used it for debugging wined3d so far, but I probably will at some point. I'm not sure it's really suited for profiling wined3d much though, other than for creating something reproducible to profile. (That's useful, but doesn't solve the entire problem.)
Using ApiTrace to improve D3D/OpenGL performance
Dear all, Just wanted to bring to your attention ApiTrace 2.0, http://zrusin.blogspot.com/2011/09/apitrace-20.html . Apparently this tool is quite fast and should allow to play back GL commands, to analyse performance and replay commands. This might be useful to understand where some bottlenecks of wined3d might be? What do you think? Cheers
design comments in code or in tests?
Hi, here's another one you WineConf folks may want to discuss. I've already expressed that there are not enough comments in mmdevapi's code. I thought about adding some, but wondered whether there's any value in using the header format that old code in Wine uses/used for automated doc. Generation (whatever the exact format was). For instance, comments ought to document GetPosition's contract in sufficient detail: - 0 <= GetPosition <= sum of written samples (modulo Frequency) - underruns don't count, it's no wall clock - monotically increasing(?) - should reflect speaker position, not buffering (MSDN says that) Don't look up MSDN. If that were complete enough, I would not have had the need to write as many tests as I did. Then I thought about placing comments in dlls/*/tests/ instead, because the test code is thrown away less often than the dll code. Seriously. Regards, Jörg Höhle
Subject: [PATCH 1/2 (try2)] mmdevapi: Don't fail if dwChannelMask is not set correctly
Hi, Andrew Eikum writes: >(try2): Remove the test found by testbot. My Win7 Home VM passed that >test, so it seems we can expect any behavior. I object to the removal of the test like that. It should be replaced with a comment to explain that the behaviour of IsFormatSupported is very surprising, hence skipped. Actually, that behaviour begs for even more tests to understand what's going on. So far I believed in the invariant: IsFormatSupported == S_OK <=> Initialize == S_OK which is verified using all standard formats (1-2 channels), with one exception on testbot: render.c:396: Initialize (1, 48000x16x1) returns 8889000f unlike IsFormatSupported ^1^ means in exclusive mode, and only there. Now you experienced A. on testbot: IsFormatSupported(dwChannelMask==) == S_OK, like Initialize which conforms to the above invariant and simply suggests that ChannelMask is not checked at all, therefore this code in Wine seems too much: +fmtex->dwChannelMask != 0 && +fmtex->dwChannelMask != get_channel_mask(pwfx->nChannels)) B. your own Win7 Home system, with dwChannelMask=, IsFormatSupported returns +ok(hr == S_FALSE || hr == AUDCLNT_E_UNSUPPORTED_FORMAT while Initialize returns +ok(hr == S_OK, which is weird. When does IFS return UNSUPPORTED_FORMAT? When does it return S_FALSE? In the latter case, is the suggested format anything else than GetMixFormat? What does exclusive mode return? Unlike in Wine, the shared mode "audio engine" mixer is a completely different SW entity than the exclusive mode "audio device", so it's important to check both. This needs more testing. In particular, tests performed on HW that supports more than 2 channels are welcome. Based on your surprising test result, I'd guess yours is such a system. Unlike DSound, mmdevapi has no GetSpeakerConfig that I'm aware of (short of Device Topology), so I don't know how apps find out about multi-channel abilities. BTW, I believe +fmtex->dwChannelMask != get_channel_mask(pwfx->nChannels)) in Wine is bogus anyway, because it's arbitrary. There exist different 3/4/5/6 channel configurations. Regards, Jörg Höhle
Re: winscard
On 9/30/2011 4:47 PM, Marcus Meissner wrote: +State = pStates[i].dwEventState& (~SCARD_STATE_CHANGED); > +rgReaderStates[i].cbAtr = pStates[i].cbAtr; > +memcpy(rgReaderStates[i].rgbAtr, pStates[i].rgbAtr, MAX_ATR_SIZE); The "MAX_ATR_SIZE" size looks wrong. Should it be perhaps pStates[i].cbAtr which seems to be the bytecount? I'll answer this since these patches are taken from my original big patch: copying cbAtr bytes makes more sens but copying MAX_ATR_SIZE is not wrong since the two buffers are of the same size and here we are just transferring all bytes from pcsclite side to the WIN32 side. That being said, maybe putting pStates[i].cbAtr will help the patches be accepted. By the way, I want to thank Vincent for taking the time to try to incorporate the winscard code into Wine. Without his dedication, this could languish for many years more! Cheers, -- Mounir IDRASSI IDRIX http://www.idrix.fr
Re: user32: Prioritize focus for dialog owner on EndDialog.
On Fri, Sep 30, 2011 at 8:36 AM, Marvin wrote: > ... > === W7PROX64 (64 bit msg) === > Timeout > > === TEST64_W7SP1 (64 bit msg) === > Timeout It seems that these 64-bit tests have been failing for a while (it's clearly not related to my patch). Is anyone already looking into this issue? If not then I'll take a crack at it. Erich Hoover ehoo...@mines.edu
Re: winscard
On Fri, Sep 30, 2011 at 11:42:26AM +0200, Vincent Hardy wrote: > Hi all, > > Here are four patches that are a partial but functional > implementation of winscard. > > I installed "Belgian Electronic Identity Card MiddleWare" > (http://eid-mw.googlecode.com/files/BeidMW35-6995.msi) > > I tested winscard by starting beid35gui.exe and I can see my > identity card data. > > Moreover, this should help to solve this bug > http://bugs.winehq.org/show_bug.cgi?id=26978. > > Are these 4 patches good enough to be sent to wine-patch ? Patch looks largely good to me. 1 comment I have on quick review below. It is probably hard to write testcases that work without hardware, but if you can think of any, they would be welcome. > +State = pStates[i].dwEventState & (~SCARD_STATE_CHANGED); > +rgReaderStates[i].cbAtr = pStates[i].cbAtr; > +memcpy(rgReaderStates[i].rgbAtr, pStates[i].rgbAtr, > MAX_ATR_SIZE); The "MAX_ATR_SIZE" size looks wrong. Should it be perhaps pStates[i].cbAtr which seems to be the bytecount? With that clarified you could already send them to wine-patches as-is. Ciao, Marcus
Re: user32: Prioritize focus for dialog owner on EndDialog.
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 http://testbot.winehq.org/JobDetails.pl?Key=14644 Your paranoid android. === W7PROX64 (64 bit msg) === Timeout === TEST64_W7SP1 (64 bit msg) === Timeout
Re: winscard
On 30/09/11 19:42, Vincent Hardy wrote: > > Are these 4 patches good enough to be sent to wine-patch ? Vincent, normally you would start by adding tests that can confirm the same behaviour in Windows. If you can do that and get the test accepted then you are a long closer to get your patches accepted too. As you start with the tests you need to code the tests for functions you have not implemented in the same patch as a todo wine so that the test will not break the wine test suite.
Re: winscard
On Fri, Sep 30, 2011 at 06:42, Vincent Hardy wrote: > Hi all, > > Here are four patches that are a partial but functional implementation of > winscard. Congratulations =) > I installed "Belgian Electronic Identity Card MiddleWare" > (http://eid-mw.googlecode.com/files/BeidMW35-6995.msi) > > I tested winscard by starting beid35gui.exe and I can see my identity card > data. I can help you test this (although currently I'm lacking the time to), I have 8 different contact and contactless readers and dozens of different card types. And several demo applications and full fledged hsm applications that run with over 200 smartcards at the same time =) > Moreover, this should help to solve this bug > http://bugs.winehq.org/show_bug.cgi?id=26978. > > Are these 4 patches good enough to be sent to wine-patch ? > > winscard is really useful for me and probably for other people too. I'm not used to patch reviewing but here are my comments: You don't need to check for invalid parameters on the functions, at least in most of them. pcsclite should do that by itself and for example in the SCardListreaders you don't need to interpret AUTOALLOCATE because pcsclite handles that [1]. In SCardConnect you are also checking and returning the invalid parameters error but pcsclite checks that [1]. Again with SCardStatus, I guess it should be as simple as a forward to pcsclite. You're defining MAX_ATR_SIZE 36 but the standard ISO 7816 value is 33 bytes and both pcsclite and windows winscard use 33. At least once you do HeapAlloc + memset zero, usually this can be done by passing a flag HEAP_ZERO_MEMORY to HeapAlloc. > > Vincent > [1] = http://pcsclite.alioth.debian.org/api/winscard__clnt_8c_source.html Best wishes, Bruno
Re: Buildbot status
On Fri, Sep 30, 2011 at 00:33, Jerome Leclanche wrote: > On Thu, Sep 29, 2011 at 8:59 PM, Dan Kegel wrote: >> Do you object to Greg's existing testbot emails, too > > Kinda, yeah. I read wine-devel for developer comments, insights, > development about wine, etc. Bot emails feel kind of wrong here; I > especially dislike WTB's because a lot of them are timeout-sort of > spam on one odd machine. Well as long as the test failures remain available from http://source.winehq.org/patches/ (e.g. http://source.winehq.org/patches/data/79447.testfail), I don't see why the list should receive automatic replies/mails. That should probably be opt-in so interested people can still receive messages on their private email address. Don't know if that's currently possible for WTB, though, or if it's a hardcoded list of recipients. Maarten probably knows better
winscard
Hi all,
Here are four patches that are a partial but functional implementation
of winscard.
I installed "Belgian Electronic Identity Card MiddleWare"
(http://eid-mw.googlecode.com/files/BeidMW35-6995.msi)
I tested winscard by starting beid35gui.exe and I can see my identity
card data.
Moreover, this should help to solve this bug
http://bugs.winehq.org/show_bug.cgi?id=26978.
Are these 4 patches good enough to be sent to wine-patch ?
winscard is really useful for me and probably for other people too.
Vincent
>From 28e9922180f6124c0387d853fecf5cf3a6c841ba Mon Sep 17 00:00:00 2001
From: Vincent Hardy
Date: Fri, 30 Sep 2011 09:57:36 +0200
Subject: [PATCH 1/4] winscard implementation (1/4)
---
configure.ac |3 ++
dlls/winscard/winscard.c | 67 ++
2 files changed, 70 insertions(+), 0 deletions(-)
mode change 100644 => 100755 configure.ac
mode change 100644 => 100755 dlls/winscard/winscard.c
diff --git a/configure.ac b/configure.ac
old mode 100644
new mode 100755
index 1401072..4b02330
--- a/configure.ac
+++ b/configure.ac
@@ -1643,6 +1643,9 @@ fi
dnl Check for libodbc
WINE_CHECK_SONAME(odbc,SQLConnect,,[AC_DEFINE_UNQUOTED(SONAME_LIBODBC,["libodbc.$LIBEXT"])])
+dnl Check for libpcsclite
+WINE_CHECK_SONAME(pcsclite,SCardEstablishContext,,[AC_DEFINE_UNQUOTED(SONAME_LIBPCSCLITE,["libpcsclite.$LIBEXT"])])
+
dnl Disable unsupported winmm drivers
test -n "$ALSALIBS" || enable_winealsa_drv=${enable_winealsa_drv:-no}
test -n "$COREAUDIO" || enable_winecoreaudio_drv=${enable_winecoreaudio_drv:-no}
diff --git a/dlls/winscard/winscard.c b/dlls/winscard/winscard.c
old mode 100644
new mode 100755
index 412299c..6d07890
--- a/dlls/winscard/winscard.c
+++ b/dlls/winscard/winscard.c
@@ -17,16 +17,36 @@
*/
#include "config.h"
+#include "wine/port.h"
#include
#include "windef.h"
#include "winbase.h"
#include "wine/debug.h"
+#include "wine/library.h"
#include "winscard.h"
#include "winternl.h"
+static BOOL PCSCLite_loadlib(void);
+static void PCSCLite_loadfunctions(void);
+
WINE_DEFAULT_DEBUG_CHANNEL(winscard);
+static LONG (*pSCardEstablishContext)(DWORD dwScope, LPCVOID pvReserved1, LPCVOID pvReserved2, LPSCARDCONTEXT phContext);
+static LONG (*pSCardIsValidContext)(SCARDCONTEXT hContext);
+static LONG (*pSCardReleaseContext)(SCARDCONTEXT hContext);
+static LONG (*pSCardListReaders)(SCARDCONTEXT hContext, LPCSTR mszGroups, LPSTR mszReaders, LPDWORD pcchReaders);
+static LONG (*pSCardCancel)(SCARDCONTEXT hContext);
+static LONG (*pSCardGetStatusChange)(SCARDCONTEXT hContext, DWORD dwTimeout, LPSCARD_READERSTATEA rgReaderStates, DWORD cReaders);
+static LONG (*pSCardStatus)(SCARDHANDLE hCard, LPSTR mszReaderNames, LPDWORD pcchReaderLen, LPDWORD pdwState, LPDWORD pdwProtocol, BYTE* pbAtr, LPDWORD pcbAtrLen);
+static LONG (*pSCardGetAttrib)(SCARDHANDLE hCard, DWORD dwAttrId, BYTE* pbAttr, LPDWORD pcbAttrLen);
+static LONG (*pSCardConnect)(SCARDCONTEXT hContext, LPCSTR szReader, DWORD dwShareMode, DWORD dwPreferredProtocols, LPSCARDHANDLE phCard, LPDWORD pdwActiveProtocol);
+static LONG (*pSCardDisconnect)(SCARDHANDLE hCard, DWORD dwDisposition);
+static LONG (*pSCardBeginTransaction)(SCARDHANDLE hCard);
+static LONG (*pSCardTransmit)(SCARDHANDLE hCard, LPCSCARD_IO_REQUEST pioSendPci, const BYTE* pbSendBuffer, DWORD cbSendLength, LPSCARD_IO_REQUEST pioRecvPci, BYTE* pbRecvBuffer, LPDWORD pcbRecvLength);
+static LONG (*pSCardEndTransaction)(SCARDHANDLE hCard, DWORD disposition);
+
static HMODULE WINSCARD_hModule;
+static void *g_pcscliteHandle;
static HANDLE g_startedEvent = NULL;
const SCARD_IO_REQUEST g_rgSCardT0Pci = { SCARD_PROTOCOL_T0, 8 };
@@ -44,6 +64,10 @@ BOOL WINAPI DllMain (HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
{
DisableThreadLibraryCalls(hinstDLL);
WINSCARD_hModule = hinstDLL;
+
+if (PCSCLite_loadlib())
+PCSCLite_loadfunctions();
+
/* FIXME: for now, we act as if the pcsc daemon is always started */
g_startedEvent = CreateEventA(NULL,TRUE,TRUE,NULL);
break;
@@ -51,6 +75,10 @@ BOOL WINAPI DllMain (HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
case DLL_PROCESS_DETACH:
{
CloseHandle(g_startedEvent);
+
+ if (g_pcscliteHandle)
+wine_dlclose(g_pcscliteHandle, NULL, 0);
+
break;
}
}
@@ -58,6 +86,45 @@ BOOL WINAPI DllMain (HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
return TRUE;
}
+static BOOL PCSCLite_loadlib(void)
+{
+char error[256];
+
+g_pcscliteHandle = wine_dlopen(SONAME_LIBPCSCLITE, RTLD_LAZY | RTLD_GLOBAL, error, sizeof(error));
+if (g_pcscliteHandle)
+return TRUE;
+else
+{
+WARN("Failed to open library %s : %s\n", debugstr_a(SONAME_LIBPCSCLITE), error);
+return FALSE;
+}
+}
+
+static void PCSCLite_loadfunctio
Re: Favorite bug: deadlock preventing backtraces?
Hey Dan, On 09/29/2011 06:00 AM, Dan Kegel wrote: > On one of my boxes, if I run winmm/midi.ok in a loop, it will > eventually deadlock/crash > ( http://bugs.winehq.org/show_bug.cgi?id=28388 ): > err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main > process heap section" wait timed out in thread 0022, blocked by 001f, > retrying (60 sec) > wine: Critical section 00110060 wait failed at address 0x7bc3484a > (thread 0022), starting debugger... > err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main > process heap section" wait timed out in thread 0022, blocked by 001f, > retrying (60 sec) > > Sadly, no backtrace follows ( http://bugs.winehq.org/show_bug.cgi?id=28265 ). > Since I'm not around when it finally happens, I haven't tried btall yet. > > I suppose I should write a shell script to watch for that error > message in the log, > and run btall then... any better ideas? > > (I did try giving winedbg a private heap, didn't seem to prevent the > deadlock.) > > I'm wondering if it's because of the lock in virtual_handle_stack_fault, what if you remove that? :P
