December 2008 - Wine-Devel - list.winehq.org

Re: mshtml: suppress module unloading while gecko is loaded

by Dan Kegel

On Mon, Dec 1, 2008 at 12:08 AM, Paul Bryan Roberts <pbronline-wine(a)yahoo.co.uk> wrote: > In mshtml, I count 7 dll reference count inc/dec pairs, including the server > lock pair, in main.c, htmldoc.c and protocol.c I count 28 pairs of > InterlockedDecrement / InterlockedIncrement calls around the creation / > destruction of active objects throughout the dll. That would seem to imply > 22 missing reference count inc/dec pairs. I suspect Alexandre didn't commit my lame patch because he wants us to have a go at getting CanDLLUnloadNow right. Thanks for the analysis. > If I finish my script, it will report how many pairs are missing for which > dlls and in which source files That sounds quite handy. > which might help make a decision whether the > take up Mike Hearn's suggestion of never unloading anything. Even that > should involve work taking out the reference counts that are already there. I have a feeling we're not quite in the "memory is free" era yet... > I do not see how the classic WinAPI regression tests can catch this kind of > bug other than being a tautologous test of the creation/destruction all the > known active objects. If Mike Hearn's suggestion is not taken up, the > script might also be useful for regression test purposes. Your script is doing static analysis ... maybe it should be integrated into http://people.redhat.com/mstefani/wine/smatch/ Making it a smatch rule might be easier than writing it from scratch. - Dan

Dec. 1, 2008

Re: netapi32: Why my patch is ignored?

by Paul Vriens

Vitaly Perov wrote: > What is wrong with my patch? > > I have fixed all errors found by Juan Lang. > I resent this patch several times, but hasn't received any answer. > > Why my patch is ignored? > > Maybe something is still wrong, but why somebody just tell me about it? > > "Juan Lang" <juan.lang(a)gmail.com> wrote: >> Hi Vitaliy, overall your patch looks pretty good. A few minor points: >> >> + FIXME("(%s %d %p) partially implemented\n", >> debugstr_w(servername), level, buf); >> Because you add a more specific FIXME for every unimplemented path, it >> seems to me this would be better as a TRACE. >> >> + result = RegCreateKeyExA(HKEY_LOCAL_MACHINE, regKey, 0, NULL, 0, >> KEY_WRITE, >> + NULL, &hKey, &disposition); >> + if (result != ERROR_SUCCESS) >> + { >> + FIXME("Unable to open/create key 'HKLM\\%s'\n", regKey); >> + return result; >> This shouldn't be a FIXME, as there's nothing to "fix" in the code: >> if the key couldn't be created, there's a setup problem or some other >> unrecoverable error. A TRACE is fine. > >> + /* FIXME: A duplicate patch checking should be implemented here */ >> Do you mean a duplicate path? > > > ------------------------------------------------------------------------ > > I'm not sure about this, but I don't like the mixture of A and W-calls. -- Cheers, Paul.

Dec. 1, 2008

Re: netapi32: Why my patch is ignored?

by Alexandre Julliard

Vitaly Perov <vitperov(a)etersoft.ru> writes: > + GetCurrentDirectoryA(MAX_PATH, tmpPath); > + result = SetCurrentDirectoryW(path); > + SetCurrentDirectoryA(tmpPath); > + if (!result) return ERROR_FILE_NOT_FOUND; This is not the right way to test if a directory exists. -- Alexandre Julliard julliard(a)winehq.org

Dec. 1, 2008

netapi32: Why my patch is ignored?

by Vitaly Perov

What is wrong with my patch? I have fixed all errors found by Juan Lang. I resent this patch several times, but hasn't received any answer. Why my patch is ignored? Maybe something is still wrong, but why somebody just tell me about it? "Juan Lang" <juan.lang(a)gmail.com> wrote: >Hi Vitaliy, overall your patch looks pretty good. A few minor points: > >+ FIXME("(%s %d %p) partially implemented\n", >debugstr_w(servername), level, buf); >Because you add a more specific FIXME for every unimplemented path, it >seems to me this would be better as a TRACE. > >+ result = RegCreateKeyExA(HKEY_LOCAL_MACHINE, regKey, 0, NULL, 0, >KEY_WRITE, >+ NULL, &hKey, &disposition); >+ if (result != ERROR_SUCCESS) >+ { >+ FIXME("Unable to open/create key 'HKLM\\%s'\n", regKey); >+ return result; >This shouldn't be a FIXME, as there's nothing to "fix" in the code: >if the key couldn't be created, there's a setup problem or some other >unrecoverable error. A TRACE is fine. >+ /* FIXME: A duplicate patch checking should be implemented here */ >Do you mean a duplicate path? -- Best wishes, Vitaly Perov Russia, Saint-Petersburg. www.etersoft.ru

Dec. 1, 2008

Mono integration to Wine

by Kornél Pál

Hi, Wine is still using Mono by creating a new process altough it has support for _CorExeMain and _CorDllMain. There is a MonoFixupCorEE method exported by mono.dll that could be used. I also did some research on executing exe assemblies without creating a mono.exe process on MS Windows by injecting code to the new process before initialization to load Mono. Note that because mscoree.dll is loaded at a very early process initilization stage no side-by-side manifests can be used that is required by newer MSVCRTs so I'll most likely create a separate DLL that will load mono.dll later in _CorExeMain or _CorDllMain. For the above mentioned reason it may be better not to use MonoFixupCorEE yet. I would like to receive comments on mixed-mode assembly support of Mono on Wine and if there are people willing to update the Loader to call _CorValidateImage, _CorExeMain (real entry point is not called for EXEs) and _CorImageUnloading for managed images. MSVCRT also should call CorExitProcess on exit. Kornél

Dec. 1, 2008

Re: [1/4] winemenubuilder: move path normalization to Process_Link and Process_URL

by Ambroz Bizjak

On Monday 01 December 2008, Francois Gouget wrote: > On Sun, 30 Nov 2008, Ambroz Bizjak wrote: > [...] > > > To allow that, I've modified winemenubuilder to record created shortcuts > > to registry, and my service will obtain and maintain the list of .lnk/url > > files from there. > > The general idea is good, but I don't think you should store these paths > in the registry. Instead the daemon should ask for file change > notifications for the relevant special folders (CSIDL_STARTMENU, etc) > and act accordingly when a .lnk or .url file gets added / deleted > somewhere in thes directories. In that case the daemon would still have to store the list of known files, so that if a shortcut is deleted while Wine is not running, or if my service is not running at that particular time, its removal could still be detected. Another issue with your suggestion is that it would complicate watching. Assuming the notification functions don't work everywhere, and they may have race conditions*, we still have to poll for changes - that means recursing the folders. Last, my service does not yet remove created icons. With the current design, it is largely a matter of also recording the icon names to registry in winemenubuilder. It would be quite hard and hackish to locate the icon from the .desktop file. It would even be hard to get the logical location of the shortcut only from the location of the .lnk, because functions in menubuilder that do that require the file to exist. * I once deleted two watched files in some folder, but only one notification was generated, and my program found only one of the files missing.

Dec. 1, 2008

Re: Did anyone have any trouble compiling wine today? Or was it just me ....

by Susan Cragin

>>I don't know off hand then. I'd say run a regression test. As a guess, >>you might try removing: >>#ifdef HAVE_LINUX_SERIAL_H >>#include <linux/serial.h> >>#endif >> >>but that would just be a kludgy workaround. >> >>-- >>-Austin > >Will try both tomorrow. >Thanks. Austin: Did not see above section, but did see this, so removed it. #ifdef HAVE_LINUX_INPUT_H #include <linux/input.h> #endif Did not work. Got same error. :(

Dec. 1, 2008

Re: [2/2] shell32/autocomplete: add unit tests for autocomplete

by Paul Vriens

Jan de Mooij wrote: > Hi Paul, > > Thanks for your reply. > > On Mon, Dec 1, 2008 at 3:35 PM, Paul Vriens <paul.vriens.wine(a)gmail.com> wrote: > >> There is an unrelated change to shelllink.c in this patch. > > It's not unrelated, initguid.h needs to be included exactly once or > you will get linker warnings (multiple definitions for...). > It only compiles if i move it to autocomplete.c, looks like it should > be in the first file which uses it.. > > Regards, > > Jan de Mooij > But I guess that's the reason it's not applied/committed. If you move the include of initguid.h to after windows.h it will work, not? -- Cheers, Paul.

Dec. 1, 2008

Re: [2/2] shell32/autocomplete: add unit tests for autocomplete

by Jan de Mooij

Hi Paul, Thanks for your reply. On Mon, Dec 1, 2008 at 3:35 PM, Paul Vriens <paul.vriens.wine(a)gmail.com> wrote: > There is an unrelated change to shelllink.c in this patch. It's not unrelated, initguid.h needs to be included exactly once or you will get linker warnings (multiple definitions for...). It only compiles if i move it to autocomplete.c, looks like it should be in the first file which uses it.. Regards, Jan de Mooij

Dec. 1, 2008

Re: [2/2] shell32/autocomplete: add unit tests for autocomplete

by Paul Vriens

Jan de Mooij wrote: > Tests pass on 98, XP and Wine (after applying my previous patch) > > > ------------------------------------------------------------------------ > > There is an unrelated change to shelllink.c in this patch. -- Cheers, Paul.

Dec. 1, 2008

Re: [1/4] winemenubuilder: move path normalization to Process_Link and Process_URL

by Francois Gouget

On Sun, 30 Nov 2008, Ambroz Bizjak wrote: [...] > To allow that, I've modified winemenubuilder to record created shortcuts to > registry, and my service will obtain and maintain the list of .lnk/url files > from there. The general idea is good, but I don't think you should store these paths in the registry. Instead the daemon should ask for file change notifications for the relevant special folders (CSIDL_STARTMENU, etc) and act accordingly when a .lnk or .url file gets added / deleted somewhere in thes directories. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Advice is what we ask for when we already know the answer but wish we didn't -- Eric Jong

Dec. 1, 2008

Fwd: [1/2] shell32/autocomplete: fix handling of WM_KILLFOCUS messages (try 2)

by Jan de Mooij

Hello, Anybody knows why this patch was rejected? Thanks in advance. ---------- Forwarded message ---------- From: Jan de Mooij <jandemooij(a)gmail.com> Date: Sat, Nov 29, 2008 at 8:57 PM Subject: [1/2] shell32/autocomplete: fix handling of WM_KILLFOCUS messages (try 2) To: wine-patches(a)winehq.org

Dec. 1, 2008

Re: wined3d: Sign-compare warnings fix

by Andrew Talbot

Austin English wrote: >> This->baseShader.device; >> int i; >> - unsigned int extra_constants_needed = 0; >> + unsigned int i, extra_constants_needed = 0; > > You forgot to remove 'int i' here. > Thanks, Austin. Good catch! -- Andy.

Dec. 1, 2008

USB device drivers support

by Alexander Morozov

Hello, I sent patch "Add support of WDM drivers for USB devices." in October and received such reply from Alexandre Julliard: > Your design needs a lot more thought. You can't add all these > Wine-specific modules, or make winedevice special-case usb devices, or > poll the server for the add_device request like you do. All this needs > to be properly integrated in the existing infrastructure. I`d like to ask about better way of modifying patches which add USB device drivers support before doing something. Wineusb and wineusbhub can be moved to usbhub.sys. This driver creates a device which is used as second argument for AddDevice on Windows XP. So no wine-specific modules will be added. Using wineserver to call AddDevice can be avoided if move code which loads device drivers and code which enumerates devices and call AddDevice to one process. This can be made if use one winedevice process for all drivers. This will allow every driver to use IRP, DEVICE_OBJECTs and other structures created by other driver and will help to implement such functions as IoGetDeviceObjectPointer. Can I do so? Or there are some reasons which do not allow? Thanks, Alexander

Dec. 1, 2008

Re: [advapi32/tests] Fix some crashes on NT4 (Try2)

by Paul Vriens

On Sat, Nov 29, 2008 at 12:52 PM, Paul Vriens <paul.vriens.wine(a)gmail.com> wrote: > Hi, > > (Try2 removes some useless LocalFree's when NewAcl is NULL, duh). > > NT4 crashes at 3 distinct places. The first two we can get rid of by > removing > the LocalFree on failure. The third one we can skip as we can detect NT4 > easily. > Just to be a little bit more precise. NT4 doesn't crash at those LocalFree's but leaving either of them in will make sure the test crashes a bit later. -- Cheers, Paul.

Dec. 1, 2008

Re: Error Message and Office 2000

by Rob Shearman

2008/11/30 SWEETSKY(a)ROCKETMAIL.COM <sweetsky(a)rocketmail.com>: > Hi, > > I'm in Southern California. > I did a search for the event when my > Office 2000 Professional gave me an error > message. It won't setup. > The event was 1000 > and the fault address is the same as a > read out of your computer that you posted online. > http://www.winehq.org/pipermail/wine-devel/2003-July/018313.html > Did you ever figure out how to get it to work? It looks like you want the wine-users mailing list. Alternatively, you could try CrossOver instead: http://www.codeweavers.com/products/cxlinux/download_trial/ -- Rob Shearman

Dec. 1, 2008

Re: ole32: implement CoGetCallContext and CoSwitchCallContext

by Rob Shearman

2008/11/30 Austin English <austinenglish(a)gmail.com>: > + if( !ppv) > + return E_INVALIDARG; > + > + /* get COM tls */ > + info = COM_CurrentInfo(); > + if( !info) Please try to keep the style consistent here. The style that the rest of the file uses is a space after the "if" and no space inside the brackets. > + return E_OUTOFMEMORY; > + > + hr = RPC_E_CALL_COMPLETE; > + if( info->ctx_call) > + { > + hr = IUnknown_QueryInterface( info->ctx_call, riid, ppv); > + } > + > + if (FAILED(hr)) ... > + /* Receives a pointer to the call context object of the call currently > + in progress. This value is returned so that the original call context > + can be restored by the custom marshaller. > + The returned pointer will be NULL if there was no call in progress. */ > + *ppOldObject = info->ctx_call; > + > + /* It may be NULL if you are calling CoSwitchCallContext to switch back > + to the original call context and there was no original call context. */ > + info->ctx_call = pNewObject; You don't take a reference to the object that is passed-in. This is against COM semantics and so requires a test case to prove that this is indeed what the function does. -- Rob Shearman

Dec. 1, 2008

Re: wineconsole: Wait on events in curses backend using select instead of poll

by Martin Storsjö

On Mon, 1 Dec 2008, Kai Blin wrote: > On Sunday 30 November 2008 00:52:57 Martin Storsjö wrote: >> poll can't handle terminal devices on Darwin, since Tiger. See the >> following discussion: >> http://lists.apple.com/archives/Darwin-dev/2006/Apr/msg00066.html > > There's reasons we switched from select to poll. You can't just switch back to As far as I can tell from the history of curses.c, it never used select (directly) before, it used WaitForMultipleObjects, which was changed into a poll in 267d38e1bc78541221d20cc72a759acc78908e8e, "wineconsole: Don't use a Win32 wait on a Unix file descriptor." But there may of course be other reasons for using poll, which I don't know about. > select to work around a OSX _bug_. If Apple can't get their act together to > fix this, this needs to go behind a configure check/ifdef. Yes, that's true. I'm not too familiar with the wine codebase and customs, so instead of nagging you with questions on what a proper solution would look like, should I just file a bug with the actual issue; that the curses based console is unusable on Darwin at the moment? // Martin

Dec. 1, 2008

Re: d3d9 : do not proceed reset or release from another thread than the one which created the device.

by Henri Verbeet

2008/11/30 Jérôme Gardou <jerome.gardou(a)gmail.com>: > - if (ref == 0) { > + if (ref <= 0) { This doesn't work, of course. ref is an ULONG. The check for Reset should probably go into wined3d.

Dec. 1, 2008

Re: mshtml: suppress module unloading while gecko is loaded

by Paul Bryan Roberts

> Dan Kegel wrote: > Thanks to Mike Hearn for pointing out that mshtml reference > counting was wrong, and Jacek for pointing out how to fix > it for now. > > Once gecko is loaded, don't let mshtml unload. > This fixes a crash in starting Sketchup (bug 16164), > and probably fixes a number of other apps that > went pear-shaped after unloading mshtml... > Bug 14065 is another instance of the same thing. I did a quick survey of DllCanUnloadNow() on the Wine 1.1.8 code base a couple of weeks ago ... 55 DllCanUnloadNow routines (in some 272 dll subdirectories) 11 FIXME stubs return S_FALSE 7 FIXME stubs return S_OK 3 without FIXME return S_FALSE 2 without FIXME return S_OK 32 appear to be real implementations However, the first in the list (alphabetically) is in astream/main.c. It's reference counter (ddl_ref) is not incremented or decremented anywhere. I count four pairs of inc/dec omissions. In mshtml, I count 7 dll reference count inc/dec pairs, including the server lock pair, in main.c, htmldoc.c and protocol.c I count 28 pairs of InterlockedDecrement / InterlockedIncrement calls around the creation / destruction of active objects throughout the dll. That would seem to imply 22 missing reference count inc/dec pairs. If I finish my script, it will report how many pairs are missing for which dlls and in which source files, which might help make a decision whether the take up Mike Hearn's suggestion of never unloading anything. Even that should involve work taking out the reference counts that are already there. I do not see how the classic WinAPI regression tests can catch this kind of bug other than being a tautologous test of the creation/destruction all the known active objects. If Mike Hearn's suggestion is not taken up, the script might also be useful for regression test purposes. Cheers,

Dec. 1, 2008

Re: wineconsole: Wait on events in curses backend using select instead of poll

by Kai Blin

On Sunday 30 November 2008 00:52:57 Martin Storsjö wrote: > poll can't handle terminal devices on Darwin, since Tiger. See the > following discussion: > http://lists.apple.com/archives/Darwin-dev/2006/Apr/msg00066.html There's reasons we switched from select to poll. You can't just switch back to select to work around a OSX _bug_. If Apple can't get their act together to fix this, this needs to go behind a configure check/ifdef. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developer http://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton.

Dec. 1, 2008

Re: X11DRV: GetKeyNameTextW returns a wrong length

by Vitaliy Margolen

Vincent Povirk wrote: >> --- keyboard.old 2008-11-27 16:53:15.000000000 +0100 >> +++ keyboard.c 2008-11-26 21:19:01.000000000 +0100 All patches have to be generated from the top of the source tree (if you not using git). This patch is invalid. Vitaliy

Nov. 30, 2008

Re: X11DRV: GetKeyNameTextW returns a wrong length

by Vincent Povirk

I don't think "lpBuffer[nSize - 1] = 0;" is safe to remove. If nSize is too small, the resulting string may not be NULL-terminated. Patch-sending guidelines are at http://www.winehq.org/site/sending_patches. Your patch is more likely to be committed if you write a conformance test (although it may be enough in this case to send a patch that obviously works, since this is documented behavior). The appropriate place for that in the wine source is probably dlls/user32/tests. See http://www.winehq.org/site/docs/winedev-guide/testing for guidelines on conformance tests. Vincent Povirk On Thu, Nov 27, 2008 at 3:42 PM, Nils Hoyer <Privat(a)nils-hoyer.de> wrote: > http://bugs.winehq.org/show_bug.cgi?id=16195#c0 > > > This is my first sending of a Wine patch - and first usage of a patch at > all. I'm sorry, if I made any mistake... > > > GetKeyNameTextW results a 1 instead of a the lengh of the Text. This patch > fixes this bug. > > --- keyboard.old 2008-11-27 16:53:15.000000000 +0100 > +++ keyboard.c 2008-11-26 21:19:01.000000000 +0100 > @@ -2328,11 +2328,7 @@ > TRACE("found scan=%04x keyc=%04x keysym=%04x string=%s\n", > scanCode, keyc, (int)keys, name); > if (lpBuffer && nSize && name) > - { > - MultiByteToWideChar(CP_UNIXCP, 0, name, -1, lpBuffer, nSize); > - lpBuffer[nSize - 1] = 0; > - return 1; > - } > + return MultiByteToWideChar(CP_UNIXCP, 0, name, -1, lpBuffer, nSize) > - 1; > } > > /* Finally issue WARN for unknown keys */ > > > >

Nov. 30, 2008