Re: for the record, my ole32 binary tree search patch is correct
On 27.10.2009 22:50, Vincent Povirk wrote: I sent the following patch recently: commit ee6856d874d687c4504914e61bcde3e6b8823bca Author: Vincent Povirk madewokh...@gmail.com Date: Fri Oct 23 13:57:42 2009 -0500 ole32: Don't use IEnumSTATSTG to search for elements of storages. We use it to do a linear search of a binary tree, which is overkill. Replace it with a simple binary search. It was accepted and caused this bug: http://bugs.winehq.org/show_bug.cgi?id=20477 It actually turned out that the mistake was in a previous patch, but it didn't matter for reading files until we started to depend on the tree to be valid. Nonetheless, I've done a test to make sure we can really rely on the binary trees in storage files to be valid. [...] This means that Windows does depend on the tree in a storage file to be correct, and Wine's behavior is now closer to Windows, even though it now fails in some cases when it used to succeed. If I've not mistaken, that uses lstricmp internally for comparing keys. Note, that wine lstr[i]cmp and ms windows lstr[i]cmp uses different sort order (see last [disabled] test in dlls/kernel32/tests/locale.c). So binary search using wine lstricmp on presored array (used windows lstricmp) will certainly fail on some files. PS looking at code one more time, current code seems already depends on lstricmp working same [in updatePropertyChain()], so already broken. It's pure luck that it is not failed already. Disclaimer: I'm not expert in wine code, looked at code very quickly, and may be misread something :-\
Re: for the record, my ole32 binary tree search patch is correct
On 28.10.2009 20:30, Vincent Povirk wrote: If I've not mistaken, that uses lstricmp internally for comparing keys. It uses lstrcmpiW, which according to MSDN can behave differently based on locale, but I don't think the Wine version does. Note, that, it is not only difference in 0x7f nls chars (somewhat expected), there are difference in ascii range too (see that test from test/locale.c). [I'm saying about difference between wine vs. windows; I'm not sure if windows lstricmpW /really/ work differently in different locales] [...] I think what saves us is that programs rarely expose storage and stream names directly to users, so the set of characters we have to deal with in practice is limited. I've looked at some .msi files - stream names contain problematic characters: '.' vs. '_' (mismatches between wine and windows; there are more such chars), '_' vs. '0' (mismatches between strcasecmp and strcoll|lstricmp). So, while problematic files maybe very rare, but not impossible. Still, we really should use a comparison function that's at least guaranteed to be consistent.
Re: Wine and XShm support
On 10.08.2009 12:24, Henri Verbeet wrote: at least add a registry key for it and disable it perhaps on 'modern drivers'). What do you guys think? Could you do some benchmarking? You can disable XShm support by passing --without-xshm to configure. Note that problem is only with *Shm*Pixmap*. *Shm*Image* is fine. So --without-xshm will disable too much. AFAIR, with really modern nvidia drivers (=180.x), Xserver should report 'ShmPixmap unsupported' in XShmQueryVersion (see driver's README about 'Option AllowSHMPixmaps'; check `xdpyinfo -ext MIT-SHM` to be sure); so new ShmPixmap code should be already no-op on nvidia and not cause pessimization ;-) [not sure about Intel and others; so, maybe, adding wine option still make sense].
Re: [rfc] lstrcmpi: order still wrong (was Re: Regression in lstrcmpiA (occurred in late June, NLS related) from 2003 year)
On 04.07.2009 23:55, Yuriy Kaminskiy wrote: Yuriy Kaminskiy wrote: I'm wrong - I don't have working windows installation at hands and cannot check that. Well, no answer so far; I thought should write test, code is more welcomed than just words, and noticed that such test already present, but disabled :-E. That's wrong. If test report breakage, it should not be simply silenced and forgotten for 6 years. See [rfc] [kernel32/tests] enable sort order test series in *-patches
Re: [rfc] lstrcmpi: order still wrong (was Re: Regression in lstrcmpiA (occurred in late June, NLS related) from 2003 year)
Yuriy Kaminskiy wrote: I've stumbled over problem with lstrcmpi sorting is still wrong. Some japanese game engine uses binary search on presorted array, and fails with a-la object not found errors. [...] proper order should be _ 0 (ok) and . _ (fails with vanilla wine). Well, after private email, I think I should stress out, that while I /believe/ that sort order in winxp does not depend on locale, it is also /possible/ that I'm wrong - I don't have working windows installation at hands and cannot check that. [Nevertheless, I've ran that game in japanese locale [ja_JP.UTF-8], and /if/ sort order in winxp depend on locale, sort order in wine should be fixed to depend on locale too: still bug, but slightly different ;-)].
[rfc] lstrcmpi: order still wrong (was Re: Regression in lstrcmpiA (occurred in late June, NLS related) from 2003 year)
Hello! Previous thread on this topic: http://www.mail-archive.com/wine-devel@winehq.org/msg01080.html I've stumbled over problem with lstrcmpi sorting is still wrong. Some japanese game engine uses binary search on presorted array, and fails with a-la object not found errors. Judging by object order in archive, === cut === ... conf_p.MGD- (would fail with strcasecmp, ok with wine) conf01.MGD--/ ... title.MGD-- fails with vanilla wine title_p.MGD--/ ... === cut === proper order should be _ 0 (ok) and . _ (fails with vanilla wine). I've replaced collation weight of '_' with 0x02560111, and now these games run fine; but that's dirty hack, of cause, and should not be applied to upstream: 1) it is modifies generated file; 2) weight for _ chosen arbitrary and can cause conflicts somewhere else (or, rather, not can, but certainly will - there are other symbols with weight 0x0256???); 3) weight for other _-like chars should be modified too. Hope you can suggest better solution. FWIW, I've checked mentioned in previous thread unicode-2.1.9d8 tables - same mismatch, will not work too. I think, only proper way is somehow extract this table from windows (either directly by LCMapStringW(LC_MAP_SORTKEY), or sorting array of a[i]=i; with CompareStringW and using that order). I'm not a lawyer, but really doubt that such reproduced table can be considered copyrightable anywhere. How can anyone make compatible reimplementation without reproducing in some way this table? --
[patch] segv on use-after-free in dsound/buffer.c
Hello!
One of games rarely crashed with segv in line 86 of dsound/buffer.c:
85: IDirectSoundBuffer_Release((LPDIRECTSOUNDBUFFER)This-dsb);
86: This-dsb-notify = NULL;
(sorry, I failed to save actual backtrace at the time). This looks
like typical assign-after-free bug. I've applied attached patch
(wine-1.1.7, now on 1.1.19), no crashes so far (btw, similar
*Secodary*Release method down in code uses proper order -
assign-NULL-then-release).
--- wine-1.1.7/dlls/dsound/buffer.c.orig 2009-03-14 15:28:10.0 +0300
+++ wine-1.1.7/dlls/dsound/buffer.c 2009-04-23 19:50:42.0 +0400
@@ -82,8 +82,8 @@ static ULONG WINAPI IDirectSoundNotifyImpl_Release(
TRACE((%p) ref was %d\n, This, ref + 1);
if (!ref) {
-IDirectSoundBuffer_Release((LPDIRECTSOUNDBUFFER)This-dsb);
This-dsb-notify = NULL;
+IDirectSoundBuffer_Release((LPDIRECTSOUNDBUFFER)This-dsb);
HeapFree(GetProcessHeap(), 0, This);
TRACE((%p) released\n, This);
}
