July 2008 - Wine-devel - list.winehq.org

user32: Add a test for ToUnicode - Win9x behaviour
by Andre Wisplinghoff 30 Jul '08

30 Jul '08

Hello, I think my patch [1] shouldn't be applied, because I didn't think about older versions of Windows. According to MSDN, ToUnicode has NT 3.1 as a minimum, meaning we should test if the function is unimplemented and skip the test in this case (Win9x Systems). Unfortunately I don't have a Win9x system ready for testing at the moment. Is adding the following after the first call to ToUnicode enough to handle this issue? if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) { trace("Skipping the ToUnicode test on a Win9x platform\n"); return; } Best Regards -- Andre Wisplinghoff [1] http://www.winehq.org/pipermail/wine-patches/2008-July/058745.html

3 5

Re: d3dx8: Implement D3DXBoxBoundProbe
by H. Verbeet 30 Jul '08

30 Jul '08

2008/7/30 David Adam <david.adam.cnrs(a)gmail.com>: > I forgot to add a change recommended by Henri. > Now, everything should be fine. > Oops, missed that myself as well :-)

1 0

Re: d3dx8: Implement D3DXBoxBoundProbe
by H. Verbeet 30 Jul '08

30 Jul '08

Looks ok to me.

1 0

Re: [PATCH] relay: Avoid reading stack memory on unprotected side of stack pointer
by Alexandre Julliard 30 Jul '08

30 Jul '08

John Reiser <jreiser(a)BitWagon.com> writes: > Thus it seems to me that both the offset and the segment are OK > in the patched code, including when the stack is in a 16-bit segment. > If this analysis has errors, then I wish to learn. You cannot assume that the context you have to restore is identical to what call_from_32_regs has saved. So yes, call_from_32_regs will always be running in 32-bit mode and will save a 32-bit context; but the restore part needs to be able to restore any arbitrary context, including one with a 16-bit stack and separate %ds,%es,%ss selectors. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: d3dx8: Implement D3DXBoxBoundProbe
by H. Verbeet 30 Jul '08

30 Jul '08

Looks good in principle, but I've still got a few comments: > + if ( praydirection->x >= 0.0f ) > + { > + tmin = ( pmin->x - prayposition->x ) / praydirection->x; > + tmax = ( pmax->x - prayposition->x ) / praydirection->x; > + } ... > + if ( tmax < 0.0f ) return FALSE; This will break if praydirection->x is -0.0, because in that case you'll end up with -INF for positive values of tmin and tmax, and INF for negative values. One way to solve this is to do the division before the if. Ie: float div; ... div = 1.0 / praydirection->x; if (div >= 0.0f) { tmin = (pmin->x - prayposition->x) * div; tmax = (pmax->x - prayposition->x) * div; } ... That way you'll test against INF or -INF rather than 0.0 or -0.0 (which are considered equal). I don't think this one is right: > + if ( tymax > tmax ) tmax = tymax; if ( tymax < tmax ) tmax = tymax; would work better I think. > + if ( tzmin > tmin ) tmin = tzmin; > + if ( tzmax > tmax ) tmax = tzmax; These are redundant. Although the code looks pretty clear to me, in general it might make sense to briefly describe the algorithm. Something along the lines of "Consider the box as the intersection of three slabs. Clip the ray against each slab, if there's anything left of the ray after we're done we've got an intersection of the ray with the box."

1 0

Re: [PATCH 1/3] mstask/test: NewWorkItem conformance test
by Dan Hipschman 30 Jul '08

30 Jul '08

Hey Roy, On Tue, Jul 29, 2008 at 04:44:45PM -0700, Roy Shea wrote: > +static BOOL setup_task_scheduler() > +{ > + HRESULT hres; > + > + hres = CoCreateInstance(&CLSID_CTaskScheduler, NULL, CLSCTX_INPROC_SERVER, > + &IID_ITaskScheduler, (void **) &test_task_scheduler); > + return hres == S_OK; > +} You should do an "ok" check on this so that if it fails, the tests fail, since this is part of the module you're testing. > +static void test_NewWorkItem() > +{ > + HRESULT hres; > + ITask *task; > + const WCHAR task_name[] = {'T', 'e', 's', 't', 'i', 'n', 'g', 0}; > + const GUID invalid_id = {0x00000000, 0x0000, 0x0000, > + {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}; You can use GUID_NULL and eliminate this local. > + if (!setup_task_scheduler()) > + { > + skip("Failed to create task scheduler. Skipping tests.\n"); > + goto out; > + } Why goto-return when you can just return? I thought we hated gotos. ;)

2 1

Re: [PATCH 2/3] mstask: Task stub with AddRef, QueryInterface, and Release
by Dan Hipschman 29 Jul '08

29 Jul '08

On Tue, Jul 29, 2008 at 04:45:31PM -0700, Roy Shea wrote: > +static HRESULT WINAPI MSTASK_ITask_GetTriggerString( > + ITask* iface, > + WORD iTrigger, > + LPWSTR *ppwszTrigger) > +{ > + FIXME("(%p, %d, %p): stub\n", iface, iTrigger, ppwszTrigger); > + FIXME("Not implemented\n"); > + return E_NOTIMPL; > +} Forgot to remove the original FIXME.

1 0

Re: [PATCH] shell32: autocomplete: there is no need for separate IAutoComplete and IAutoComplete2 vtbls
by Juan Lang 29 Jul '08

29 Jul '08

Hi Mikolaj, + if (IsEqualIID(riid, &IID_IUnknown) || + IsEqualIID(riid, &IID_IAutoComplete) || + IsEqualIID(riid, &IID_IAutoComplete)) Perhaps you mean IID_IAutoComplete2 for one of these? --Juan

1 0

Re: Missed patches concerning valgrind-detected issues
by Kirill K. Smirnov 29 Jul '08

29 Jul '08

В сообщении от 29 июля 2008 Alexandre Julliard написал(a): > "Kirill K. Smirnov" <lich(a)math.spbu.ru> writes: > > Hello, Alexandre, > > > > dsound: check error status of waveOutGetVolume() function before using > > retrieved values (found by valgrind) > > http://www.winehq.org/pipermail/wine-patches/2008-July/058124.html > > You should most likely propagate the errors instead of assuming default > values. It seems that it is a bigger problem than I expected (just because OSS works OK here). Please, ignore this patch. > > cmd: Do not deceive CompareString() passing more characters then we > > really have (found by valgrind) > > http://www.winehq.org/pipermail/wine-patches/2008-July/058504.html > > There shouldn't be any need to pass an explicit length here, the code is > confused. Hmm... We are just trying to perform strncmpi() here - so we obligated to pass an explicit length. Unfortunately there is not such simple function a-la lstrncmpi() in WinAPI (I've double checked this!) I believe strncmpiW() function from winelib is more suitable - but I'm not sure that it will not introduce new bugs and any unexpected behaviour (diacritic weights, etc). Thanks -- Kirill

1 0

Re: [WideChar conversion improvements - 1/5] Add a new check for 'dstlen < 0' to WideCharToMultiByte
by Alexandre Julliard 29 Jul '08

29 Jul '08

"Colin Finck" <mail(a)colinfinck.de> writes: > @@ -1893,7 +1893,7 @@ INT WINAPI WideCharToMultiByte( UINT page, DWORD flags, LPCWSTR src, INT srclen, > const union cptable *table; > int ret, used_tmp; > > - if (!src || (!dst && dstlen)) > + if (!src || (!dst && dstlen) || dstlen < 0) There are places in Wine that use a negative dstlen, you have to fix them first. -- Alexandre Julliard julliard(a)winehq.org

2 1