July 2015 - Wine-Devel - list.winehq.org

Re: psapi/tests: Don't check for exact value (try 2)

by Sebastian Lackner

On 23.07.2015 22:50, André Hentschel wrote: > --- > dlls/psapi/tests/psapi_main.c | 59 +++++++++++++------------------------------ > 1 file changed, 17 insertions(+), 42 deletions(-) > > diff --git a/dlls/psapi/tests/psapi_main.c b/dlls/psapi/tests/psapi_main.c > index cebfc5f..1ee30d2 100644 > --- a/dlls/psapi/tests/psapi_main.c > +++ b/dlls/psapi/tests/psapi_main.c > @@ -194,6 +194,11 @@ static void test_GetModuleInformation(void) > ok(info.lpBaseOfDll == hMod, "lpBaseOfDll=%p hMod=%p\n", info.lpBaseOfDll, hMod); > } > > +#define ADMITTED_ERROR (64) > +#define CHECK_MARGIN(perf, sys) \ > + ok(perf >= max(sys, ADMITTED_ERROR) - ADMITTED_ERROR && perf <= sys + ADMITTED_ERROR, \ > + "expected approximately " #perf "=%u but got %u\n", sys, (ULONG)perf) > + > static void test_GetPerformanceInfo(void) > { > PERFORMANCE_INFORMATION info; > @@ -227,36 +232,16 @@ static void test_GetPerformanceInfo(void) > ok(status == STATUS_SUCCESS, "expected STATUS_SUCCESS, got %08x\n", status); > ok(size >= sizeof(SYSTEM_PERFORMANCE_INFORMATION), "incorrect length %d\n", size); > > - ok(info.CommitTotal == sys_performance_info->TotalCommittedPages, > - "expected info.CommitTotal=%u but got %u\n", > - sys_performance_info->TotalCommittedPages, (ULONG)info.CommitTotal); > - > - ok(info.CommitLimit == sys_performance_info->TotalCommitLimit, > - "expected info.CommitLimit=%u but got %u\n", > - sys_performance_info->TotalCommitLimit, (ULONG)info.CommitLimit); > - > - ok(info.CommitPeak == sys_performance_info->PeakCommitment, > - "expected info.CommitPeak=%u but got %u\n", > - sys_performance_info->PeakCommitment, (ULONG)info.CommitPeak); > - > - ok(info.PhysicalAvailable >= max(sys_performance_info->AvailablePages, 25) - 25 && > - info.PhysicalAvailable <= sys_performance_info->AvailablePages + 25, > - "expected approximately info.PhysicalAvailable=%u but got %u\n", > - sys_performance_info->AvailablePages, (ULONG)info.PhysicalAvailable); > + CHECK_MARGIN(info.CommitTotal, sys_performance_info->TotalCommittedPages); > + CHECK_MARGIN(info.CommitLimit, sys_performance_info->TotalCommitLimit); > + CHECK_MARGIN(info.CommitPeak, sys_performance_info->PeakCommitment); > + CHECK_MARGIN(info.PhysicalAvailable, sys_performance_info->AvailablePages); > > /* TODO: info.SystemCache not checked yet - to which field(s) does this value correspond to? */ > > - ok(info.KernelTotal == sys_performance_info->PagedPoolUsage + sys_performance_info->NonPagedPoolUsage, > - "expected info.KernelTotal=%u but got %u\n", > - sys_performance_info->PagedPoolUsage + sys_performance_info->NonPagedPoolUsage, (ULONG)info.KernelTotal); > - > - ok(info.KernelPaged == sys_performance_info->PagedPoolUsage, > - "expected info.KernelPaged=%u but got %u\n", > - sys_performance_info->PagedPoolUsage, (ULONG)info.KernelPaged); > - > - ok(info.KernelNonpaged == sys_performance_info->NonPagedPoolUsage, > - "expected info.KernelNonpaged=%u but got %u\n", > - sys_performance_info->NonPagedPoolUsage, (ULONG)info.KernelNonpaged); > + CHECK_MARGIN(info.KernelTotal, sys_performance_info->PagedPoolUsage + sys_performance_info->NonPagedPoolUsage); > + CHECK_MARGIN(info.KernelPaged, sys_performance_info->PagedPoolUsage); > + CHECK_MARGIN(info.KernelNonpaged, sys_performance_info->NonPagedPoolUsage); > > /* compare with values from SYSTEM_BASIC_INFORMATION */ > size = 0; > @@ -264,13 +249,8 @@ static void test_GetPerformanceInfo(void) > ok(status == STATUS_SUCCESS, "expected STATUS_SUCCESS, got %08x\n", status); > ok(size >= sizeof(SYSTEM_BASIC_INFORMATION), "incorrect length %d\n", size); > > - ok(info.PhysicalTotal == sys_basic_info.MmNumberOfPhysicalPages, > - "expected info.PhysicalTotal=%u but got %u\n", > - sys_basic_info.MmNumberOfPhysicalPages, (ULONG)info.PhysicalTotal); > - > - ok(info.PageSize == sys_basic_info.PageSize, > - "expected info.PageSize=%u but got %u\n", > - sys_basic_info.PageSize, (ULONG)info.PageSize); > + CHECK_MARGIN(info.PhysicalTotal, sys_basic_info.MmNumberOfPhysicalPages); I think we can safely assume that the total physical memory does not change between two calls. > + CHECK_MARGIN(info.PageSize, sys_basic_info.PageSize); The pagesize should usually also match exactly. > > /* compare with values from SYSTEM_PROCESS_INFORMATION */ > size = 0; > @@ -297,14 +277,9 @@ static void test_GetPerformanceInfo(void) > } > HeapFree(GetProcessHeap(), 0, sys_process_info); > > - ok(info.HandleCount == handle_count, > - "expected info.HandleCount=%u but got %u\n", handle_count, info.HandleCount); > - > - ok(info.ProcessCount == process_count, > - "expected info.ProcessCount=%u but got %u\n", process_count, info.ProcessCount); > - > - ok(info.ThreadCount == thread_count, > - "expected info.ThreadCount=%u but got %u\n", thread_count, info.ThreadCount); > + CHECK_MARGIN(info.HandleCount, handle_count); > + CHECK_MARGIN(info.ProcessCount, process_count); > + CHECK_MARGIN(info.ThreadCount, thread_count); A tolerance of 64 (especially for process/threads) basically disables these tests. A smaller tolerance would already be sufficient to avoid random test failures. > } > } > >

July 26, 2015

Re: D3DRM Implementation patches upto CreateDeviceFromSurface

by Aaryaman Vasishta

Here's my current work on CreateDeviceFromClipper. If this is good enough, the CreateDeviceFromSurface, after which CreateDeviceFromD3D implementations will follow. On Thu, Jul 9, 2015 at 8:04 PM, Aaryaman Vasishta <jem456.vasishta(a)gmail.com > wrote: > > > On Thu, Jul 9, 2015 at 5:20 PM, Stefan Dösinger <stefandoesinger(a)gmail.com > > wrote: > >> Is there really a need to store (and AddRef) the clipper? The primary >> surface already keeps a reference to it, so you should be OK. It won't give >> you the increment by 2 that native has though, but I don't think that's >> important. >> > At the moment there doesn't seem to be a significant need, except for the > refcounting requirements. > >> >> > + hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &ds, >> NULL); >> > + if (FAILED(hr)) >> > + return hr; >> > + >> > + hr = IDirectDrawSurface_AddAttachedSurface(surface, ds); >> > + if (FAILED(hr)) >> > + { >> > + IDirectDrawSurface_Release(ds); >> > + return hr; >> > + } >> Similarly: Is there a need to store the depth stencil? You could just >> release your reference after attaching it to the render target, it will >> stay alive anyway. (If it's just for the DeleteAttachedSurface later: >> Ignore it) >> > It's just for the DeleteAttachedSurface part. I'll remove it for now so > it's easier to thunk towards version 3. > >> >> You have to keep ddraw alive because nothing (neither your surfaces nor >> your device) hold a reference to it. Same for the primary. I think it's >> also good to store a reference to the render target, I believe you need it >> to show the rendering results on the screen (The immediate mode device >> holds a ref too, but it's awkward to always query it from the device). >> >> > + if (device->ref == 1 && device->z_created_internally) >> > + >> IDirectDrawSurface_DeleteAttachedSurface(device->render_target, 0, >> device->depth_surface); >> As mentioned on IRC this is not thread safe A lot can happen between your >> ref == 1 comparison and the actual InterlockedDecrement. >> > Since we're not gonna use DeleteAttachedSurface, we won't need to read the > refcount either. I agree there are some synchronization issues in this > case. A good idea for future reference would be to use flags to determine > which version the device was created on, to make it thread-safe and generic > enough for the release method to be used by all versions. > >> >> Wrt patch 6 (d3drm1_CreateDeviceFromClipper): It's a bit unfortunate that >> this is mostly a copypasted version of version 3, and the difference that >> actually matters is in init_device anyway. Maybe there is a nicer solution, >> e.g. a helper function for v1 and v3 with a version parameter. >> > Similar to set_primary_surface defined above init_device in the patches > (though we won't need the version parameter there anymore as I'll be using > the struct there.). > > > I've taken note of all the other suggestions and will incorporate them in > the next patchset. I do agree there's quite a bit of optimization left with > regards to the thunking and version checks. > > > Thanks! > Jam > >

July 25, 2015

Re: user32/tests: Also check against the low word of hSubMenu (try 2)

by Marvin

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 https://testbot.winehq.org/JobDetails.pl?Key=15565 Your paranoid android. === w2003std (32 bit menu) === menu.c:2545: Test failed: id 202: expected fState 0, got 0003 menu.c:2545: Test failed: id 202: expected fState 0, got 1000

July 25, 2015

Re: [PATCH 14/21] api-ms-win-crt-multibyte-l1-1-0: Add stub dll.

by Martin Storsjö

Hi, On Fri, 24 Jul 2015, Sebastian Lackner wrote: > On 23.07.2015 21:47, Martin Storsjo wrote: >> +@ stdcall __p__mbcasemap() ucrtbase.__p__mbcasemap > > This (and a couple of other redirects) should probably be declared as cdecl, similar to how its done in all other msvcr* dlls. > You have to update the hack in tools/make_specfiles for this to work properly, for example like that: > > --- snip --- > @@ -376,7 +376,7 @@ sub update_spec_file($) > if ($parent{callconv} ne "stub" || $parent{args}) > { > my $callconv = $parent{callconv} ne "stub" ? $parent{callconv} : > - $parent{spec} =~ /msvc/ ? "cdecl" : "stdcall"; # hack > + $parent{spec} =~ /(msvc|ucrtbase)/ ? "cdecl" : "stdcall"; # hack > $_ = sprintf "$descr{ordinal} %s %s%s", $callconv, $flags, $func; > > if ($parent{target} =~ /$group_head\./) # use the same forward as parent if possible > --- snip --- Thanks, will do. Perhaps vcruntime also should be added here? I'll await more comments before resubmitting the patchset since it's pretty large... // Martin

July 25, 2015

Re: [PATCH 14/21] api-ms-win-crt-multibyte-l1-1-0: Add stub dll.

by Sebastian Lackner

On 23.07.2015 21:47, Martin Storsjo wrote: > +@ stdcall __p__mbcasemap() ucrtbase.__p__mbcasemap This (and a couple of other redirects) should probably be declared as cdecl, similar to how its done in all other msvcr* dlls. You have to update the hack in tools/make_specfiles for this to work properly, for example like that: --- snip --- @@ -376,7 +376,7 @@ sub update_spec_file($) if ($parent{callconv} ne "stub" || $parent{args}) { my $callconv = $parent{callconv} ne "stub" ? $parent{callconv} : - $parent{spec} =~ /msvc/ ? "cdecl" : "stdcall"; # hack + $parent{spec} =~ /(msvc|ucrtbase)/ ? "cdecl" : "stdcall"; # hack $_ = sprintf "$descr{ordinal} %s %s%s", $callconv, $flags, $func; if ($parent{target} =~ /$group_head\./) # use the same forward as parent if possible --- snip --- Please note that this is the first patch affected by this problem.

July 24, 2015

Re: Packaging wine-development on Debian

by Nathan Schulte

On 07/24/2015 03:27 AM, Kyle Auble wrote: > On 07/23/2015 04:29 PM, Nathan Schulte wrote: >> It seems like the packaging in Debian of both Wine Stable and Wine >> Development as different "packages" in their own right was the wrong >> choice. > > So I looked into it a little more, and it's arguably not so much a > choice as the natural result of an improvement to the packaging on > Debian. Yes, poor word choice on my part. Thanks for digging into this. Michael's reply is insightful as well; it definitely sounds like this isn't an entirely unknown issue. I look forward to the discussion. -- Nate

July 24, 2015

Re: user32: Hide horizontal Listbox scroll bar if no horizontal extent is set.

by Joachim Priesner

Am Freitag, 24. Juli 2015 schrieb Marvin: > 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 > https://testbot.winehq.org/JobDetails.pl?Key=15534 > > Your paranoid android. > > > === wvistau64 (32 bit listbox) === > listbox.c:709: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 > listbox.c:944: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 > > === wvistau64 (64 bit listbox) === > listbox.c:709: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 > listbox.c:944: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 Unrelated failures.

July 24, 2015

Re: [PATCH 1/2] user32: Initialize buffer before sending WM_GETTEXT

by Nikolay Sivov

On 24.07.2015 3:13, Marvin wrote: > 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 > https://testbot.winehq.org/JobDetails.pl?Key=15535 > > Your paranoid android. > > > === wvistau64 (32 bit win) === > win.c:7546: Test failed: transparent window didn't get WM_NCHITTEST message > win.c:7547: Test failed: button under static window didn't get WM_LBUTTONUP > > This is unrelated.

July 24, 2015

Wine Stable (Was: Re: [website] Update Debian downloads info (Resend))

by Michael Stefaniuc

On 07/24/2015 01:29 AM, Nathan Schulte wrote: > On 07/23/2015 05:23 PM, Kyle Auble wrote: >> I migrated to Debian Stable within the past year (still think Ubuntu's >> great, just a minimalist that wanted to wander a bit upstream). If I >> understand everything, I think the main reason the wine team still >> bothers with a stable release is for distros like Debian Stable or >> Redhat. > > Does Wine treat Stable this way? In that it receives bugfixes/security > updates (from Development, mainline) that are applicable to making it > more stable/secure? From the release log, it looks like Stable is > basically dead; the last release was well over a year ago. I think with We already have a topic "Wine Stable Considered Harmful" for the WineConf in September, see http://wiki.winehq.org/WineConf2015 > this scenario, as you hint at with your closing remarks Kyle, if there > was a single release (no Stable/Development), Debian would just choose > not to package the newer version and only release the one it believes is > most stable (perhaps with Debian curated bugfix/security updates from > upstream). It seems Stable is to help facilitate this (awesome!), but > it doesn't actually work that way in practice. On a high level view this are IMHO the good and bad things about the current stable process in Wine: Good Bad ---- --- Code freeze/stabilization Way too old Marketing Release slips with features slippage Some distributions want it Distributions insist on using it Bug reports for stable are ignored bye michael

July 24, 2015

Packaging wine-development on Debian

by Kyle Auble

On 07/23/2015 04:29 PM, Nathan Schulte wrote: > On 07/23/2015 05:23 PM, Kyle Auble wrote: >> Sorry for the goofy formatting. One day Thunderbird doesn't wrap >> anything for me, the next it's cutting my lines short. I'm resending >> my message, hopefully with better formatting this time. > As a fellow Icedove user, I can say that you're not the only one who > runs up against this. It's especially hairy for me around re-wrapping > quoted content; I think I just don't understand how it works. I think that sometimes the packagers or developers change default settings between releases; I used to have that all the time with Firefox on Ubuntu. Apparently by default, the current Jessie version of Icedove wraps plain-text emails at 72 characters once you send them, but automatically unwraps them if you read them. You have to go to the config editor if you want to change that. >> If I understand everything, I think the main reason the wine team >> still bothers with a stable release is for distros like Debian Stable >> or Redhat. > Does Wine treat Stable this way? In that it receives > bugfixes/security updates (from Development, mainline) that are > applicable to making it more stable/secure? From the release log, it > looks like Stable is basically dead; the last release was well over a > year ago. The impression I get is that the wine project itself treats stable a little differently. Unless someone finds an egregious security issue in wine itself, it seems like the consensus is to keep wine-stable as inert as possible and let packagers apply their own minor patches on top. It could be because so many features are still being improved, and there's still an expectation that the development release is more likely to work for your program anyway. Also, since wine works with so many other components on the system, it might be hard to assign blame to wine or the environment alone in a lot of cases. In that situation, it kind of makes sense to delegate minor, post-release fixes to the distros. >> I wonder though why the maintainers went with a separate >> wine-development package instead of just pointing the wine package in >> testing & unstable to the development release, then offering that to >> stable through Backports. > We can always loop them in and ask: > pkg-wine-party(a)lists.alioth.debian.org > > It seems like the packaging in Debian of both Wine Stable and Wine > Development as different "packages" in their own right was the wrong > choice. So I looked into it a little more, and it's arguably not so much a choice as the natural result of an improvement to the packaging on Debian. Apparently, there was traditionally a wine-unstable package in testing & unstable, it would conflict with an install of stable wine, and its packaging code is separate from the stable wine package's. The Debian packagers decided in the past year or so to change the name of wine-unstable to wine-development. At the same time though, someone put in a lot of work and managed to make the two packages capable of coinstallation via update-alternatives. This bug report gives a pretty good overview: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=758291 I guess after doing that, they figured, "Why not offer both packages for Jessie? They don't conflict." I'm still going to ask on the mail-list about possibly having a Backports version though (plus I have some other questions related to multi-arch). While the difference between wine-development and wine is a little confusing at first, I don't think that's the real problem. In my mind, putting wine-development in the stable repo is self-defeating; you lose most of a development branch's value if it doesn't track upstream closely, and wine-development on Jessie is still stuck in last October. - Kyle

July 24, 2015