November 2009 - Wine-Devel - list.winehq.org

today's git broke winetricks gecko :-(
by Joerg-Cyril.Hoehle＠t-systems.com Nov. 30, 2009

Nov. 30, 2009

Hi, >wine cmd /c echo yes now downloads gecko. Me to -- I'm not pleased with Wine-1.1.33 starting an installation request upon startup. - How am I going to automate regression testing when it hangs waiting for a mouse click? - I tried to think positively about it and expected the winetest-1.1.33.exe to install Gecko and perform the mshtml tests when online. Curiously they were skipped. I have yet to find out why. >Any suggestions are welcome. Winecfg would be a better place to propose the installation of an optional component. Perhaps I'm a very atypical Wine user: - no MMORPG, no ie6 - My Wine is not used for anything online. Quite to the contrary, I have some iptables firewall rules to disable Wine IP traffic should anything ever manage to get in. - I never used winetricks. I sometimes peeked at it. When I needed Gecko long ago, I saw that the Wine source contains code to install it, downloaded Gecko.cab myself, modified the one registry entry to point to the .cab and let it install. Worked. IMHO, no general-purpose application should attempt to talk to the internet when it starts. Please leave that behaviour to viruses and trojans only (and alikes, e.g. Adobe's pdf). Maybe I'm too old-fashioned in this decade of "always online even in the subway" technical achievements. Or I've been in the privacy-protection (Datenschutz) and security business for too long. Regarding winecfg, it could display "no Gecko/HTML support, click here to install it (need be online)" in flashing red to attract the user's attention once you start it. I wouldn't mind. Regards, Jörg Höhle.

13 25

Re: wineserver: Added support for read/only Registry, similar to unprivileged users (try 2)
by Alexandre Julliard Nov. 30, 2009

Nov. 30, 2009

Paul Chitescu <paulc(a)voip.null.ro> writes: > Changelog: > wineserver: Added support for read/only Registry, similar to > unprivileged users > > Rationale: > This is a first step towards implementing shared wine prefixes with > most users being unprivileged and a single Administrator (power user, etc.) > capable of installing programs. I don't think it's a step in the right direction. What you need is proper access control with security descriptors, like we have for other objects. A single read-only bit cannot be extended to support this. -- Alexandre Julliard julliard(a)winehq.org

2 3

Re: comctl32: Implementation the WM_GETTEXT processing of datetime control
by Nikolay Sivov Nov. 30, 2009

Nov. 30, 2009

Ilya Shpigor wrote: > +static INT > +DATETIME_GetText (DATETIME_INFO *infoPtr, INT count, LPWSTR dst) > +{ > + WCHAR buf[80]; > + int i; > + > + if(!count) return 0; > + > + dst[0] = 0; > + for (i = 0; i < infoPtr->nrFields; i++) > + { > + DATETIME_ReturnTxt(infoPtr, i, buf, sizeof(buf)/sizeof(buf[0])); > + if ((strlenW(dst) + strlenW(buf)) <= count) > + strcatW(dst, buf); > + else break; > + } > + return strlenW(dst); > +} > I don't think it's a right way. You probably should use window text instead updating it on every change, see how GetWindowText is implemented. > @@ -674,6 +675,10 @@ static void test_wm_set_get_text(void) > ret = SendMessage(hWnd, WM_GETTEXT, sizeof(buff), (LPARAM)buff); > ok(strcmp(buff, a_str) != 0, "Expected text not to change, got %s\n", buff); > > + GetSystemTime(&stime); > + sprintf(time, "%d.%d.%d", stime.wDay, stime.wMonth, stime.wYear); > + ok(!strcmp(buff, time), "Expected %s, got %s\n", time, buff); > + > DestroyWindow(hWnd); > } > Test is definitely wrong. You can't expect DD.MMMM.YYYY pattern here, it's locale dependent.

2 4

Re: mciqtz32: Fix open/close omissions.
by Dmitry Timoshkov Nov. 30, 2009

Nov. 30, 2009

You need to provide more information why is this change required, and what is the problem you are trying to fix. -- Dmitry.

1 0

Re: Implement a stub for PrintWindow in user32
by Dmitry Timoshkov Nov. 30, 2009

Nov. 30, 2009

"Luke Benstead" <kazade(a)gmail.com> wrote: > +/************************************************************************ > + * PrintWindow (USER32.@) > + * > + */ > +BOOL WINAPI PrintWindow(HWND hwnd, HDC hdcBlt, UINT nFlags) > +{ > + return TRUE; > +} If this is a stub, it should not return TRUE, should print a FIXME, and probably set an appropriate last error. Besides, MSDN has a nice hint how to implement this API properly. -- Dmitry.

1 0

Re: wine.inf: Remove obsolete and duplicate entries.
by Paul Vriens Nov. 30, 2009

Nov. 30, 2009

On 11/30/2009 02:08 PM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Dear reviewers, > > Vitaliy Margolen wrote in the discussion of bug #20693: >> Those entries shouldn't be in the wine.inf in the first place >> (as Alexandre already said). If anything they should be removed >>from there to lessen the confusion. > > Here is a patch to remove those entries. Note that > I'm not familiar enough with the printing system or MS-Windows' > expected vital entries to judge whether the entries should be present > as default fallbacks on systems compiled without cups, > or which have no printer, > or because some apps on MS-Windows crash if they are not present > even when there's no printer > or whatever reason. I just trust your word. > > All I can see is that these entries about [HKLM\HARDWARE] do not make > it into system.reg on my Linux machine even though they are present in > wine.inf, I suppose because the dynamic device detection code > (hal/dbus) removes them (with no printer nor cups-dev installed). > > Regards, > Jörg Höhle > Hi Jörg, This was already (partly) done in commit d19e2ada9ce2d52d8b09e984fe5b12803bf6d4ca. -- Cheers, Paul.

1 0

Re: mciavi: Simplify play loop.
by Paul Vriens Nov. 30, 2009

Nov. 30, 2009

On 11/30/2009 01:51 PM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Hi, > > in the future, MCIAVI will hopefully be able to drive the loop > with either audio or video turned on or off individually > (setaudio/setvideo off), like native (e.g. mciavi can play > the audio tracks only). > Here's a first step towards unification. > > Regards, > Jörg Höhle > Tabs and space mixtures here as well. -- Cheers, Paul.

1 0

Re: mciqtz32: Fix open/close omissions.
by Paul Vriens Nov. 30, 2009

Nov. 30, 2009

On 11/30/2009 01:47 PM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Regards, > Jörg Höhle > Hi Jörg, You're mixing tabs and spaces here. -- Cheers, Paul.

1 0

Re: [1/3] WineD3D: Infrastructure to render swapchains to a FBO
by Henri Verbeet Nov. 30, 2009

Nov. 30, 2009

2009/11/30 Stefan Dösinger <stefan(a)codeweavers.com>: > I think this will break color_fill_fbo(). My general feeling is that I'm seeing to much variants of "if (swapchain->render_to_fbo)". Also, I think this patch has parts that can be separated like e.g. the surface_init_sysmem() bit. Out of curiosity, what would the performance impact be of always rendering to FBO?

2 2

Re: gdi32 : Correction of bug 20850 Write buffer overflow in WidenPath()
by Dan Kegel Nov. 29, 2009

Nov. 29, 2009

On Sat, Nov 28, 2009 at 5:04 PM, Laurent Vromman <laurent(a)vromman.org> wrote: > - pStrokes = HeapAlloc(GetProcessHeap(), 0, numStrokes * > sizeof(GdiPath*)); > - pStrokes[0] = HeapAlloc(GetProcessHeap(), 0, sizeof(GdiPath)); > - PATH_InitGdiPath(pStrokes[0]); > - pStrokes[0]->pFlags = HeapAlloc(GetProcessHeap(), 0, > pPath->numEntriesUsed * sizeof(INT)); > - pStrokes[0]->pPoints = HeapAlloc(GetProcessHeap(), 0, > pPath->numEntriesUsed * sizeof(POINT)); > - pStrokes[0]->numEntriesUsed = 0; > + pStrokes = NULL; ... Wait, wouldn't it be simpler to keep the initial alloc, as in the attachment? - Dan

3 4

RFC: EM_FORMATRANGE Patch for Wine-1.1.33
by James McKenzie Nov. 29, 2009

Nov. 29, 2009

All: Time to beat on this patch again and make it better. I've also attached it to bug 6254. Please make comments constructive. I would like to get assistance where to break this down so that it will be 'easier to swallow' pieces as this is one huge patch that AJ has rejected before. James McKenzie

1 0

windows shutdown process
by Vincent Povirk Nov. 29, 2009

Nov. 29, 2009

In order to find the correct fix for bug 18753, I've been examining the shutdown process on Windows. I have written a program that logs the messages sent to it, and I've run it on Windows 7. What I found was that a shutdown produces the following sequence of events: * We get a 0x3b message. DefWindowProc sends a WM_QUERYENDSESSION message. We return 1, allowing the session to end. * We get another 0x3b message, which leads to a WM_ENDSESSION. * Nothing else happens, but for some reason the program exits. I added a trace after the message loop returns to see if the program gets a WM_QUIT. It doesn't. Apparently, my test program was killed. I tried changing the window proc so that it loops infinitely when it gets WM_ENDSESSION. Windows 7 sees this as "blocking shutdown". (Naturally, it responds the same way if I return 0 from WM_QUERYENDSESSION.) This leads me to believe that Windows 7, when shutting down, immediately kills any process that returns from a WM_ENDSESSION message. Does that seem sane? I've posted my test program (source code and .exe file) at http://madewokherd.nfshost.com/msgtest.zip, and I'd like to hear if other Windows versions behave the same way Windows 7 does. -- Vincent Povirk

2 2

Re: kernel32.dll: Reduce Registry access to KEY_READ or MAXIMUM_ALLOWED wherever possible (try 2)
by Rob Shearman Nov. 28, 2009

Nov. 28, 2009

2009/11/27 Paul Chitescu <paulc(a)voip.null.ro>: > --- ./dlls/kernel32/locale.c.orig 2009-11-02 05:45:10.000000000 +0200 > +++ ./dlls/kernel32/locale.c 2009-11-26 19:35:39.000000000 +0200 > @@ -3017,7 +3017,7 @@ > RtlInitUnicodeString( &keyName, szKeyName ); > InitializeObjectAttributes(&attr, &keyName, 0, hRootKey, NULL); > > - if (NtOpenKey( &hkey, KEY_ALL_ACCESS, &attr ) != STATUS_SUCCESS) > + if (NtOpenKey( &hkey, MAXIMUM_ALLOWED, &attr ) != STATUS_SUCCESS) > hkey = 0; > > return hkey; This is ugly. The function should be changed to allow the required access rights to be specified. -- Rob Shearman

1 0

Re: services.exe: Reduce Registry access to KEY_READ or MAXIMUM_ALLOWED wherever possible
by Rob Shearman Nov. 28, 2009

Nov. 28, 2009

2009/11/27 Paul Chitescu <paulc(a)voip.null.ro>: > @@ -376,7 +376,7 @@ > InitializeCriticalSection(&(*db)->cs); > > err = RegCreateKeyExW(HKEY_LOCAL_MACHINE, SZ_SERVICES_KEY, 0, NULL, > - REG_OPTION_NON_VOLATILE, KEY_ALL_ACCESS, NULL, > + REG_OPTION_NON_VOLATILE, MAXIMUM_ALLOWED, NULL, > &(*db)->root_key, NULL); > if (err != ERROR_SUCCESS) > HeapFree(GetProcessHeap(), 0, *db); I'm not happy with this change because the code was written with the assumption that data it will always have permission to write data back to keys under root_key. The return value from save_service_config isn't always checked and thus this could lead to silent data loss. In any event, the use of MAXIMUM_ALLOWED should not be encouraged where at all possible [1] and instead the code should try the most permissions it needs and fall back to less as appropriate. > @@ -417,7 +417,7 @@ > break; > > WINE_TRACE("Loading service %s\n", wine_dbgstr_w(szName)); > - err = RegOpenKeyExW(db->root_key, szName, 0, KEY_READ | KEY_WRITE, &hServiceKey); > + err = RegOpenKeyExW(db->root_key, szName, 0, KEY_READ, &hServiceKey); > if (err == ERROR_SUCCESS) > { > err = load_service_config(hServiceKey, entry); This change is fine since hServiceKey is only every used with operations that require KEY_READ at the most. -- Rob Shearman [1] http://blogs.msdn.com/larryosterman/archive/2004/09/14/229658.aspx

1 0

Re: advapi32.dll: Reduce Registry access to MAXIMUM_ALLOWED wherever possible
by Rob Shearman Nov. 28, 2009

Nov. 28, 2009

2009/11/27 Paul Chitescu <paulc(a)voip.null.ro>: > Changelog: > advapi32.dll: Reduce Registry access to MAXIMUM_ALLOWED wherever possible > > Makes read operation succeed on read/only Registry keys. Needs tests to show that this behaviour matches that on Windows. You should be able to achieve this by setting an appropriate security descriptor on the registry key before calling the function you are changing. However, the change to get_special_root_hkey looks fine to me. The rest of your patch isn't easy to review because you haven't generated the diff with the "-p" option. -- Rob Shearman

1 0

Wine programs - replicating bad design/bugs
by Jerome Leclanche Nov. 28, 2009

Nov. 28, 2009

Ok, posting here to get a broader feedback on the issue. Recently, another bug* was closed about an enhancement to a wine program - a minor feature not present on Windows. This is an issue I've seen brought up before, and which I've brought up myself**. As far as the win32 api goes, I can totally agree on dysfunctions that need to be replicated. However I find this unnecessary for the wine programs (notepad, wordpad, regedit, any or more). The idea behind bug 20844 was to have a better integration with the desktop, where Shift-F3 is a common keybind for "Search previous...". The only problem with this kind of mindset is "where should we stop with new features?". IMHO, the issue behind the two bugs linked below is only a matter of better integration, better design. I can also understand how some developers' goal is to close bugs, sometimes disregarding the fact there is a problem to deal with before then. * http://bugs.winehq.org/show_bug.cgi?id=20844 ** http://bugs.winehq.org/show_bug.cgi?id=18622 J. Leclanche PS. In bug 18622, I'm not talking about a case-insensitive registry, but for the key being actually renamed.

2 2

Re: Corrections for signal handlers
by Markus Elfring Nov. 28, 2009

Nov. 28, 2009

> And don't ask me to actually think about what I'm doing, I am still open for further discussions. > it's too hard to figure out how to print a string and a number without using printf(). This data display (e. g. for the input command and line number) is generally not a problem. But the situation is different in the context of a signal handler because of the limited set of async-signal-safe functions. Regards, Markus

1 0

Re: Corrections for signal handlers
by Markus Elfring Nov. 28, 2009

Nov. 28, 2009

> Original printed more useful information. I am aware of the desire to get some data for easier program debugging conveniently. > Just dropping it in sake of "correctness" is wrong. I have got an opposite opinion. I am curious if more software developers would like to care for complete async-signal-safety. > You'll have to come up with something that prints the same This will not happen at the moment because of conflicting expectations. > or don't touch it. I assume that the involved implementation details will be reconsidered some day in the future. Regards, Markus

2 1

Re: Corrections for signal handlers
by Vitaliy Margolen Nov. 28, 2009

Nov. 28, 2009

Markus Elfring wrote: > Hello, > > Some signal handler implementations are also affected by technical details that are described in an article by Justin Pincar. > https://www.securecoding.cert.org/confluence/display/seccode/SIG30-C.+Call+… > Your code looks even worse then your arguments for "correct" codding practices. > -static int watchdog; > +static sig_atomic_t watchdog; Don't do this, you've been told to leave it alone. > - fprintf( stderr, "wineserver crashed, please enable coredumps (ulimit -c unlimited) and restart.\n"); > + char const message[] = "wineserver crashed, please enable coredumps (ulimit -c unlimited) and restart.\n"; Should be "static const char" in that exact order. > + > + (void) write(STDERR_FILENO, message, sizeof(message) - 1); What's "(void)" doing here? Yes we do ignore returned states here and in 10000s or other places. Don't do that. > + (void) fsync(STDERR_FILENO); Why do you need sync here? > abort(); > --- a/tools/wmc/wmc.c Wine fix per program please. You'll have to split this into 3 separate patches. > static void segvhandler(int sig) > { > -<----->fprintf(stderr, "\n%s:%d: Oops, segment violation\n", input_name, line_number); > -<----->fflush(stdout); > -<----->fflush(stderr); > + char const message[] = "\nOops, segment violation\n"; Same as above - should be "static const char" in that exact order. > + > + (void) write(STDERR_FILENO, message, sizeof(message) - 1); > + (void) fsync(STDOUT_FILENO); > + (void) fsync(STDERR_FILENO); Remove bogus "(void)". > <----->abort(); > } Original printed more useful information. Just dropping it in sake of "correctness" is wrong. You'll have to come up with something that prints the same or don't touch it. Original used tabs (8-spaces) your patch uses 4-space indentation. Either keep the file's formatting or stick to 4-space indentation but DO NOT mix and match both inside one function. Same applies to tools/wrc/wrc.c Vitaliy.

1 0

re: Corrections for signal handlers
by Dan Kegel Nov. 28, 2009

Nov. 28, 2009

I'm not sure this change does what you think: - fflush(stdout); ... + (void) fsync(STDOUT_FILENO); The real problem with your change is that *we don't care* whether the output of wineserver is garbled, because it is low priority debugging information, and I've never seen it garbled in practice. In short, it's not worth the time reviewing the patch. So while we appreciate your effort to improve the correctness of Wine's code, we would prefer it if you chose a more useful target. Also, Wine's patch review system is reputation-based. Your best avenue for getting pedantic corrections accepted is to get some pragmatic improvements accepted first. You might then be able to use some of your earned karma points on your favorite bikeshed problem. - Dan

1 0

Whitespace cleanups
by Nate Gallaher Nov. 28, 2009

Nov. 28, 2009

I'm looking at working on a new area of wine (for me) and the file I'm going to be working with is rife with whitespace issues. Literal tabs are sprinkled around and the 80-col limit has been broken badly and often in easily fixable ways. I'm wondering what the acceptable approach is to these issues. I'd like to fix these issues before I start submitting functional patches. My question is this: Is it acceptable to fix the whole file in one patch (80-cols), or should I just fix the one function I'm planning on touching? Or should I grit my teeth and ignore the style issues entirely? Thanks, ~Nate

6 9

dlls/kernel32/path.c: Fix return values for UNC pathnames in GetLongPathNameW
by Alexandre Hardy Nov. 28, 2009

Nov. 28, 2009

The MSDN: http://msdn.microsoft.com/en-us/library/aa364980(VS.85).aspx states that GetLongPathNameW returns the length of the string required to hold the pathname if the buffer is too small to hold the path. In the case where the buffer is too small, the filename should not be stored in the buffer. This ensures that the buffer is not written to (for UNC pathnames) if the buffer is not large enought to store the path. Kind regards Alexandre -- -------------------------------------------------- Alexandre Hardy http://www.ahardy.za.net

2 1

Commit 05c788ac6f464b75c2a55c847a91d3a79261ca8e causes tooltip functions in e-Sword to stop functioning
by James McKenzie Nov. 27, 2009

Nov. 27, 2009

All Commit 05c788ac6f464b75c2a55c847a91d3a79261ca8e richedit: Used ITextHost methods to avoid using window handle directly caused the tooltips popups to stop functioning in e-Sword. Has this been corrected in later Wine releases? I am at wine-1.1.14 with integrating the EM_FORMATRANGE patch. Thank you. James McKenzie

4 4

Re: [4/4] ntdll: partially support HEAP_DISABLE_COALESCE_ON_FREE, allows us to detect use-after-free
by Dan Kegel Nov. 27, 2009

Nov. 27, 2009

Don't apply this one yet, just found a problem with it. On Tue, Nov 24, 2009 at 6:20 PM, Dan Kegel <dank(a)kegel.com> wrote: > This adds something like support for > HEAP_DISABLE_COALESCE_ON_FREE. > If the GlobalFlags registry entry has the > 200000 bit set, freed blocks are set > aside for a while rather than immediately > being released for reuse. This allows > us to poison them and check for use-after-free > or double-free errors, and report them as heap corruption. > > If you're running Valgrind, you get an immediate error > and stack trace for any of these heap corruption events, > which was the motivation for the patch series. > But the patches are useful even without valgrind. > > The number of blocks kept out of circulation is > 5000 by default, but can be set via an environment > variable. It ought to have a limit on number of bytes, > too, but this seemed enough for the moment. >

1 0

New valgrind results, this time with overrun checks and lots of new errors.
by Dan Kegel Nov. 26, 2009

Nov. 26, 2009

http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/ is the first full run with the heap tail check enabled. Here are the first few new problems it found. Somehow, it found a bunch of invalid reads in http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/vg-advapi32_crypt.txt all in a function called test_incorrect_api_usage(). Offhand that seems like something to suppress. The invalid write I already posted about is in http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/diff-advapi32_lsa.txt http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/vg-advapi32_lsa.txt http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/diff-advpack_files.txt http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/vg-advpack_files.txt show an off-by-one buffer size issue (forgot to include space for nul char) in advpack (fix should be easy, anybody can grab this): Invalid write of size 2 at strcpyW (unicode.h:224) by lstrcpyW (string.c:104) by AdvInstallFileW (files.c:283) by AdvInstallFileA (files.c:216) by test_AdvInstallFile (files.c:513) Address 0x7f03d12e is 0 bytes after a block of size 30 alloc'd at notify_alloc (heap.c:279) by RtlAllocateHeap (heap.c:1521) by AdvInstallFileW (files.c:282) by AdvInstallFileA (files.c:216) by test_AdvInstallFile (files.c:513) http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/diff-comctl32_tab.txt http://kegel.com/wine/valgrind/logs/2009-11-18-21.51/vg-comctl32_tab.txt show a more inscrutable error: Invalid write of size 4 at TAB_SetCurSel (tab.c:255) by TAB_WindowProc (tab.c:3367) by ??? (library.h:159) by call_window_proc (winproc.c:469) by WINPROC_CallProcAtoW (winproc.c:1023) by CallWindowProcA (winproc.c:2299) by tabSubclassProcess (tab.c:404) by ??? (library.h:159) by call_window_proc (winproc.c:469) by WINPROC_call_window (winproc.c:2223) by call_window_proc (message.c:1635) by send_message (message.c:2482) by SendMessageA (message.c:2627) by test_getters_setters (tab.c:752) by func_tab (tab.c:1174) by run_test (test.h:535) by main (test.h:585) Address 0x7f044098 is not stack'd, malloc'd or (recently) free'd Nikolay touched that code back in March, perhaps he should have a look. etc. etc. Lots of triage to do. It'd be nice if somebody could volunteer to go through these, file bugs, and assign provision blame using 'git blame'... - Dan

3 7